[PATCH] iio_trigger_find_by_name: Skip trailing newline if available

[PATCH] iio_trigger_find_by_name: Skip trailing newline if available

[Hennerich, Michael]

Skip trailing newline if available.

From: Michael Hennerich <Michael.Hennerich@analog.com>

Index: drivers/staging/iio/industrialio-trigger.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- drivers/staging/iio/industrialio-trigger.c  (revision 8368)
+++ drivers/staging/iio/industrialio-trigger.c  (working copy)
@@ -155,6 +155,9 @@
        struct iio_trigger *trig;
        bool found =3D false;

+       if (len && name[len - 1] =3D=3D '\n')
+               len--;
+
        mutex_lock(&iio_trigger_list_lock);
        list_for_each_entry(trig, &iio_trigger_list, list) {
                if (strncmp(trig->name, name, len) =3D=3D 0) {

------------------------------------------------------------------
********* Analog Devices GmbH              Open Platform Solutions
**  *****
**     ** Wilhelm-Wagenfeld-Strasse 6
**  ***** D-80807 Munich
********* Germany
Registergericht M=FCnchen HRB 40368,  Gesch=E4ftsf=FChrer: Thomas Wessel, W=
illiam A. Martin, Margaret K. Seif

[PATCH] iio_trigger_find_by_name: Skip trailing newline if available

[Hennerich, Michael]

Skip trailing newline if available.

From: Michael Hennerich <Michael.Hennerich@analog.com>

Index: drivers/staging/iio/industrialio-trigger.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- drivers/staging/iio/industrialio-trigger.c  (revision 8368)
+++ drivers/staging/iio/industrialio-trigger.c  (working copy)
@@ -155,6 +155,9 @@
        struct iio_trigger *trig;
        bool found =3D false;

+       if (len && name[len - 1] =3D=3D '\n')
+               len--;
+
        mutex_lock(&iio_trigger_list_lock);
        list_for_each_entry(trig, &iio_trigger_list, list) {
                if (strncmp(trig->name, name, len) =3D=3D 0) {

------------------------------------------------------------------
********* Analog Devices GmbH              Open Platform Solutions
**  *****
**     ** Wilhelm-Wagenfeld-Strasse 6
**  ***** D-80807 Munich
********* Germany
Registergericht M=FCnchen HRB 40368,  Gesch=E4ftsf=FChrer: Thomas Wessel, W=
illiam A. Martin, Margaret K. Seif

Re: [PATCH] iio_trigger_find_by_name: Skip trailing newline if available

[Jonathan Cameron]

Michael,

You probably want to add your Signed-off-by to this.

Acked-by: Jonathan Cameron <jic23@cam.ac.uk>


> Skip trailing newline if available.
>=20
> From: Michael Hennerich <Michael.Hennerich@analog.com>
>=20
> Index: drivers/staging/iio/industrialio-trigger.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- drivers/staging/iio/industrialio-trigger.c  (revision 8368)
> +++ drivers/staging/iio/industrialio-trigger.c  (working copy)
> @@ -155,6 +155,9 @@
>         struct iio_trigger *trig;
>         bool found =3D false;
>=20
> +       if (len && name[len - 1] =3D=3D '\n')
> +               len--;
> +
>         mutex_lock(&iio_trigger_list_lock);
>         list_for_each_entry(trig, &iio_trigger_list, list) {
>                 if (strncmp(trig->name, name, len) =3D=3D 0) {
>=20
> ------------------------------------------------------------------
> ********* Analog Devices GmbH              Open Platform Solutions
> **  *****
> **     ** Wilhelm-Wagenfeld-Strasse 6
> **  ***** D-80807 Munich
> ********* Germany
> Registergericht M=FCnchen HRB 40368,  Gesch=E4ftsf=FChrer: Thomas Wes=
sel, William A. Martin, Margaret K. Seif
>=20
>=20

Re: [PATCH] iio_trigger_find_by_name: Skip trailing newline if available

[Jonathan Cameron]

On 03/08/10 09:31, Hennerich, Michael wrote:
> Skip trailing newline if available.
>=20
> From: Michael Hennerich <Michael.Hennerich@analog.com>

Dear Michael,

Just found this one whilst trying to sort out a clean iio tree.
(sorry I completely failed to reply originally)

Anyhow, I agree with the principal that we should be ignoring the
newline.  Actually we should be ignoring whitespace before and after
the string we are actually after.  Perhaps a cleaner approach
is to use sscanf to pull out the string alone?  I don't think
we need to allow for whitespace in the trigger name (though obviously
it should be documented if we do not).  Technically I believe
(via the convenient lwn article last week on the topic) we should
ignore white space but return -EINVAL if there is say one word, some
white space and then another word...  Not sure if there is a particular=
ly
clean way to do that however.

What do you think?

Jonathan
>=20
> Index: drivers/staging/iio/industrialio-trigger.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- drivers/staging/iio/industrialio-trigger.c  (revision 8368)
> +++ drivers/staging/iio/industrialio-trigger.c  (working copy)
> @@ -155,6 +155,9 @@
>         struct iio_trigger *trig;
>         bool found =3D false;
>=20
> +       if (len && name[len - 1] =3D=3D '\n')
> +               len--;
> +
>         mutex_lock(&iio_trigger_list_lock);
>         list_for_each_entry(trig, &iio_trigger_list, list) {
>                 if (strncmp(trig->name, name, len) =3D=3D 0) {
>=20
> ------------------------------------------------------------------
> ********* Analog Devices GmbH              Open Platform Solutions
> **  *****
> **     ** Wilhelm-Wagenfeld-Strasse 6
> **  ***** D-80807 Munich
> ********* Germany
> Registergericht M=FCnchen HRB 40368,  Gesch=E4ftsf=FChrer: Thomas Wes=
sel, William A. Martin, Margaret K. Seif
>=20
>=20
>=20

RE: [PATCH] iio_trigger_find_by_name: Skip trailing newline if available

[Hennerich, Michael]

Jonathan Cameron wrote on 2010-03-22:
> On 03/08/10 09:31, Hennerich, Michael wrote:
>> Skip trailing newline if available.
>>
>> From: Michael Hennerich <Michael.Hennerich@analog.com>
>
> Dear Michael,
>
> Just found this one whilst trying to sort out a clean iio tree.
> (sorry I completely failed to reply originally)
>
> Anyhow, I agree with the principal that we should be ignoring the
> newline.  Actually we should be ignoring whitespace before and after
> the string we are actually after.

Well - in most cases people use #echo so there shouldn't be a problem with =
white space before and after.
However the newline is going to bug in case you don't use #echo -n.
I've seen many sysfs_set functions also stripping the '\n' from the string.

> Perhaps a cleaner approach is to
> use sscanf to pull out the string alone?

sscanf() is certainly a alternative here.

> I don't think we need to
> allow for whitespace in the trigger name (though obviously it should
> be documented if we do not).  Technically I believe (via the
> convenient lwn article last week on the topic) we should ignore white
> space but return -EINVAL if there is say one word, some white space
> and then another word...  Not sure if there is a particularly clean
> way to do that however.
>
> What do you think?

I agree - Don't allow whitespace in the trigger name.

Michael

>
> Jonathan
>>
>> Index: drivers/staging/iio/industrialio-trigger.c
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> --- drivers/staging/iio/industrialio-trigger.c  (revision 8368)
>> +++ drivers/staging/iio/industrialio-trigger.c  (working copy)
>> @@ -155,6 +155,9 @@
>>         struct iio_trigger *trig;
>>         bool found =3D false;
>> +       if (len && name[len - 1] =3D=3D '\n')
>> +               len--;
>> +
>>         mutex_lock(&iio_trigger_list_lock);
>>         list_for_each_entry(trig, &iio_trigger_list, list) {
>>                 if (strncmp(trig->name, name, len) =3D=3D 0) {
>> ------------------------------------------------------------------
>> ********* Analog Devices GmbH              Open Platform Solutions
>> **  *****
>> **     ** Wilhelm-Wagenfeld-Strasse 6
>> **  ***** D-80807 Munich
>> ********* Germany
>> Registergericht M=FCnchen HRB 40368,  Gesch=E4ftsf=FChrer: Thomas Wessel=
,
>> William A. Martin, Margaret K. Seif
>>
>>
>>
Greetings,
Michael

Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 4036 Geschaeft=
sfuehrer Thomas Wessel, William A. Martin, Margaret Seif

Re: [PATCH] iio_trigger_find_by_name: Skip trailing newline if available

[Jonathan Cameron]

On 03/23/10 12:23, Hennerich, Michael wrote:
> Jonathan Cameron wrote on 2010-03-22:
>> On 03/08/10 09:31, Hennerich, Michael wrote:
>>> Skip trailing newline if available.
>>>
>>> From: Michael Hennerich <Michael.Hennerich@analog.com>
>>
>> Dear Michael,
>>
>> Just found this one whilst trying to sort out a clean iio tree.
>> (sorry I completely failed to reply originally)
>>
>> Anyhow, I agree with the principal that we should be ignoring the
>> newline.  Actually we should be ignoring whitespace before and after
>> the string we are actually after.
> 
> Well - in most cases people use #echo so there shouldn't be a problem with white space before and after.
> However the newline is going to bug in case you don't use #echo -n.
> I've seen many sysfs_set functions also stripping the '\n' from the string.
Actually most uses beyond the debugging drivers stage are likely to be via
userspace libraries, but admittedly these aren't going to have excess white
space so it doesn't matter!


> 
>> Perhaps a cleaner approach is to
>> use sscanf to pull out the string alone?
> 
> sscanf() is certainly a alternative here.
> 
>> I don't think we need to
>> allow for whitespace in the trigger name (though obviously it should
>> be documented if we do not).  Technically I believe (via the
>> convenient lwn article last week on the topic) we should ignore white
>> space but return -EINVAL if there is say one word, some white space
>> and then another word...  Not sure if there is a particularly clean
>> way to do that however.
>>
>> What do you think?
> 
> I agree - Don't allow whitespace in the trigger name.

Alright, I don't really care how we do it, though agree dealing with the newline
is useful, so please send this onto Greg KH (if you haven't?) 

Jonathan

[PATCH] iio_trigger_find_by_name: Skip trailing newline if available

[michael.hennerich]

Skip trailing newline if available.

Signed-off-by: Michael Hennerich <Michael.Hennerich@analog.com>
Acked-by: Jonathan Cameron <jic23@cam.ac.uk>

 drivers/staging/iio/industrialio-trigger.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/iio/industrialio-trigger.c b/drivers/staging/iio/industrialio-trigger.c
index 693ebc4..5f76fc1 100644
--- a/drivers/staging/iio/industrialio-trigger.c
+++ b/drivers/staging/iio/industrialio-trigger.c
@@ -155,6 +155,9 @@ struct iio_trigger *iio_trigger_find_by_name(const char *name, size_t len)
 	struct iio_trigger *trig;
 	bool found = false;
 
+	if (len && name[len - 1] == '\n')
+		len--;
+
 	mutex_lock(&iio_trigger_list_lock);
 	list_for_each_entry(trig, &iio_trigger_list, list) {
 		if (strncmp(trig->name, name, len) == 0) {