Newline characters will be taken into account for the firmware search
path parameter, warn users about that and provide an example using 'echo
-n' such that it clarifies the typical use of that parameter.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 Documentation/driver-api/firmware/fw_search_path.rst | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/driver-api/firmware/fw_search_path.rst b/Documentation/driver-api/firmware/fw_search_path.rst
index a360f1009fa3..d7cb1e8f0076 100644
--- a/Documentation/driver-api/firmware/fw_search_path.rst
+++ b/Documentation/driver-api/firmware/fw_search_path.rst
@@ -22,5 +22,10 @@ can use the file:
 
 * /sys/module/firmware_class/parameters/path
 
-You would echo into it your custom path and firmware requested will be
-searched for there first.
+You would echo into it your custom path and firmware requested will be searched
+for there first. Be aware that newline characters will be taken into account
+and may not produce the intended effects. For instance you might want to use:
+
+echo -n /path/to/script > /sys/module/firmware_class/parameters/path
+
+to ensure that your script is being used.
-- 
2.25.1

Florian Fainelli <f.fainelli@gmail.com> writes:

> Newline characters will be taken into account for the firmware search
> path parameter, warn users about that and provide an example using 'echo
> -n' such that it clarifies the typical use of that parameter.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  Documentation/driver-api/firmware/fw_search_path.rst | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/driver-api/firmware/fw_search_path.rst b/Documentation/driver-api/firmware/fw_search_path.rst
> index a360f1009fa3..d7cb1e8f0076 100644
> --- a/Documentation/driver-api/firmware/fw_search_path.rst
> +++ b/Documentation/driver-api/firmware/fw_search_path.rst
> @@ -22,5 +22,10 @@ can use the file:
>  
>  * /sys/module/firmware_class/parameters/path
>  
> -You would echo into it your custom path and firmware requested will be
> -searched for there first.
> +You would echo into it your custom path and firmware requested will be searched
> +for there first. Be aware that newline characters will be taken into account
> +and may not produce the intended effects. For instance you might want to use:
> +
> +echo -n /path/to/script > /sys/module/firmware_class/parameters/path
> +
> +to ensure that your script is being used.

So I have no problem with applying this, but I have to ask...might it
not be better to fix the implementation of that sysfs file to strip
surrounding whitespace from the provided path?  This patch has the look
of a lesson learned the hard way; rather than codifying this behavior
into a feature, perhaps we could just make the next person's life a bit
easier...?

Thanks,

jon

On 4/10/2023 3:43 PM, Jonathan Corbet wrote:
> Florian Fainelli <f.fainelli@gmail.com> writes:
> 
>> Newline characters will be taken into account for the firmware search
>> path parameter, warn users about that and provide an example using 'echo
>> -n' such that it clarifies the typical use of that parameter.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>   Documentation/driver-api/firmware/fw_search_path.rst | 9 +++++++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/driver-api/firmware/fw_search_path.rst b/Documentation/driver-api/firmware/fw_search_path.rst
>> index a360f1009fa3..d7cb1e8f0076 100644
>> --- a/Documentation/driver-api/firmware/fw_search_path.rst
>> +++ b/Documentation/driver-api/firmware/fw_search_path.rst
>> @@ -22,5 +22,10 @@ can use the file:
>>   
>>   * /sys/module/firmware_class/parameters/path
>>   
>> -You would echo into it your custom path and firmware requested will be
>> -searched for there first.
>> +You would echo into it your custom path and firmware requested will be searched
>> +for there first. Be aware that newline characters will be taken into account
>> +and may not produce the intended effects. For instance you might want to use:
>> +
>> +echo -n /path/to/script > /sys/module/firmware_class/parameters/path
>> +
>> +to ensure that your script is being used.
> 
> So I have no problem with applying this, but I have to ask...might it
> not be better to fix the implementation of that sysfs file to strip
> surrounding whitespace from the provided path?  This patch has the look
> of a lesson learned the hard way; rather than codifying this behavior
> into a feature, perhaps we could just make the next person's life a bit
> easier...?

I was not sure whether it was on purpose or not, Greg, will we break 
anyone's use case if we strip off \n from the firmware path passed via 
sysfs?
-- 
Florian

On Mon, Apr 10, 2023 at 04:12:32PM -0700, Florian Fainelli wrote:
> 
> 
> On 4/10/2023 3:43 PM, Jonathan Corbet wrote:
> > Florian Fainelli <f.fainelli@gmail.com> writes:
> > 
> > > Newline characters will be taken into account for the firmware search
> > > path parameter, warn users about that and provide an example using 'echo
> > > -n' such that it clarifies the typical use of that parameter.
> > > 
> > > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> > > ---
> > >   Documentation/driver-api/firmware/fw_search_path.rst | 9 +++++++--
> > >   1 file changed, 7 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/Documentation/driver-api/firmware/fw_search_path.rst b/Documentation/driver-api/firmware/fw_search_path.rst
> > > index a360f1009fa3..d7cb1e8f0076 100644
> > > --- a/Documentation/driver-api/firmware/fw_search_path.rst
> > > +++ b/Documentation/driver-api/firmware/fw_search_path.rst
> > > @@ -22,5 +22,10 @@ can use the file:
> > >   * /sys/module/firmware_class/parameters/path
> > > -You would echo into it your custom path and firmware requested will be
> > > -searched for there first.
> > > +You would echo into it your custom path and firmware requested will be searched
> > > +for there first. Be aware that newline characters will be taken into account
> > > +and may not produce the intended effects. For instance you might want to use:
> > > +
> > > +echo -n /path/to/script > /sys/module/firmware_class/parameters/path
> > > +
> > > +to ensure that your script is being used.
> > 
> > So I have no problem with applying this, but I have to ask...might it
> > not be better to fix the implementation of that sysfs file to strip
> > surrounding whitespace from the provided path?  This patch has the look
> > of a lesson learned the hard way; rather than codifying this behavior
> > into a feature, perhaps we could just make the next person's life a bit
> > easier...?
> 
> I was not sure whether it was on purpose or not, Greg, will we break
> anyone's use case if we strip off \n from the firmware path passed via
> sysfs?

I do not know, sorry.

Greg KH <gregkh@linuxfoundation.org> writes:

> On Mon, Apr 10, 2023 at 04:12:32PM -0700, Florian Fainelli wrote:
>> I was not sure whether it was on purpose or not, Greg, will we break
>> anyone's use case if we strip off \n from the firmware path passed via
>> sysfs?
>
> I do not know, sorry.

I would be amazed if anybody is putting newlines into their firmware
path; that would be kind of a silly thing to do.

That said, I've been amazed before.

I'll go ahead and apply the docs patch, but it still doesn't really seem
like the right fix to me.

Thanks,

jon

On 4/11/23 15:20, Jonathan Corbet wrote:
> Greg KH <gregkh@linuxfoundation.org> writes:
> 
>> On Mon, Apr 10, 2023 at 04:12:32PM -0700, Florian Fainelli wrote:
>>> I was not sure whether it was on purpose or not, Greg, will we break
>>> anyone's use case if we strip off \n from the firmware path passed via
>>> sysfs?
>>
>> I do not know, sorry.
> 
> I would be amazed if anybody is putting newlines into their firmware
> path; that would be kind of a silly thing to do.
> 
> That said, I've been amazed before.
> 
> I'll go ahead and apply the docs patch, but it still doesn't really seem
> like the right fix to me.

I will submit a patch that strips off newlines from sysfs provided 
firmware paths and if this turns out to break someone's use case it 
could still be reverted, sounds good?
-- 
Florian

On Tue, Apr 11, 2023 at 03:21:31PM -0700, Florian Fainelli wrote:
> On 4/11/23 15:20, Jonathan Corbet wrote:
> > Greg KH <gregkh@linuxfoundation.org> writes:
> > 
> > > On Mon, Apr 10, 2023 at 04:12:32PM -0700, Florian Fainelli wrote:
> > > > I was not sure whether it was on purpose or not, Greg, will we break
> > > > anyone's use case if we strip off \n from the firmware path passed via
> > > > sysfs?
> > > 
> > > I do not know, sorry.
> > 
> > I would be amazed if anybody is putting newlines into their firmware
> > path; that would be kind of a silly thing to do.
> > 
> > That said, I've been amazed before.
> > 
> > I'll go ahead and apply the docs patch, but it still doesn't really seem
> > like the right fix to me.
> 
> I will submit a patch that strips off newlines from sysfs provided firmware
> paths and if this turns out to break someone's use case it could still be
> reverted, sounds good?

Sounds good.