| Message ID | 5011F15A.3060007@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2012-07-26 at 21:39 -0400, Len Brown wrote: > ...both give the number of chars in the string > without the '\0', as strncmp() wants, > but sizeof() is compile-time. I thought gcc optimised strlen("string literal") into a compile time constant too. It does in a little userspace test I just wrote, but I didn't look at its behaviour with the kernel's strlen. Ian. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 30, 2012 at 4:26 PM, Ian Campbell <ijc@hellion.org.uk> wrote: > I thought gcc optimised strlen("string literal") into a compile time > constant too. It does in a little userspace test I just wrote, but I > didn't look at its behaviour with the kernel's strlen. It depends whether you use the built-in strlen() or not. AFAIK the kernel does not use the built-in variant.
On Thu 2012-07-26 21:39:38, Len Brown wrote: > ...both give the number of chars in the string > without the '\0', as strncmp() wants, > but sizeof() is compile-time. What about introducing something like streq() to do this automatically? This is ugly.... #define streq(a, b) ... if (_buildin_constant(b)) ... ? > - if (!strncmp(val, "enable", strlen("enable"))) { > + if (!strncmp(val, "enable", sizeof("enable") - 1)) { Pavel
Len Brown <lenb@kernel.org> writes: > ...both give the number of chars in the string > without the '\0', as strncmp() wants, > but sizeof() is compile-time. Does this actually change anything? The compiler is able to expand strlen at compile time if the argument is a constant, provided that that builtin strlen isn't disabled. Andreas.
On Mon, 6 Aug 2012, Pavel Machek wrote: > On Thu 2012-07-26 21:39:38, Len Brown wrote: > > ...both give the number of chars in the string > > without the '\0', as strncmp() wants, > > but sizeof() is compile-time. > > What about introducing something like streq() to do this > automatically? This is ugly.... > > #define streq(a, b) ... if (_buildin_constant(b)) ... > > ? > > > - if (!strncmp(val, "enable", strlen("enable"))) { > > + if (!strncmp(val, "enable", sizeof("enable") - 1)) { While you're at it, there's no point using strncmp when you know the length of one of the strings beforehand. Just use memcmp, and don't subtract 1 from the sizeof value. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
06.08.2012 18:36, Alan Stern ?????: > On Mon, 6 Aug 2012, Pavel Machek wrote: > >> On Thu 2012-07-26 21:39:38, Len Brown wrote: >>> ...both give the number of chars in the string >>> without the '\0', as strncmp() wants, >>> but sizeof() is compile-time. >> >> What about introducing something like streq() to do this >> automatically? This is ugly.... >> >> #define streq(a, b) ... if (_buildin_constant(b)) ... >> >> ? >> >>> - if (!strncmp(val, "enable", strlen("enable"))) { >>> + if (!strncmp(val, "enable", sizeof("enable") - 1)) { > > While you're at it, there's no point using strncmp when you know the > length of one of the strings beforehand. Just use memcmp, and don't > subtract 1 from the sizeof value. http://www.gossamer-threads.com/lists/engine?do=post_attachment;postatt_id=41157;list=linux :)
On Mon, 6 Aug 2012, Pavel Vasilyev wrote: > 06.08.2012 18:36, Alan Stern ?????: > > On Mon, 6 Aug 2012, Pavel Machek wrote: > > > >> On Thu 2012-07-26 21:39:38, Len Brown wrote: > >>> ...both give the number of chars in the string > >>> without the '\0', as strncmp() wants, > >>> but sizeof() is compile-time. > >> > >> What about introducing something like streq() to do this > >> automatically? This is ugly.... > >> > >> #define streq(a, b) ... if (_buildin_constant(b)) ... > >> > >> ? > >> > >>> - if (!strncmp(val, "enable", strlen("enable"))) { > >>> + if (!strncmp(val, "enable", sizeof("enable") - 1)) { > > > > While you're at it, there's no point using strncmp when you know the > > length of one of the strings beforehand. Just use memcmp, and don't > > subtract 1 from the sizeof value. > > http://www.gossamer-threads.com/lists/engine?do=post_attachment;postatt_id=41157;list=linux Interestingly, many (all?) of the changes in that patch are wrong because they don't try to match the terminating '\0'. As a result, they will match against extensions of the target string as well as the target string itself. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
06.08.2012 20:28, Alan Stern ?????: > On Mon, 6 Aug 2012, Pavel Vasilyev wrote: > >> 06.08.2012 18:36, Alan Stern ?????: >>> On Mon, 6 Aug 2012, Pavel Machek wrote: >>> >>>> On Thu 2012-07-26 21:39:38, Len Brown wrote: >>>>> ...both give the number of chars in the string >>>>> without the '\0', as strncmp() wants, >>>>> but sizeof() is compile-time. >>>> >>>> What about introducing something like streq() to do this >>>> automatically? This is ugly.... >>>> >>>> #define streq(a, b) ... if (_buildin_constant(b)) ... >>>> >>>> ? >>>> >>>>> - if (!strncmp(val, "enable", strlen("enable"))) { >>>>> + if (!strncmp(val, "enable", sizeof("enable") - 1)) { >>> >>> While you're at it, there's no point using strncmp when you know the >>> length of one of the strings beforehand. Just use memcmp, and don't >>> subtract 1 from the sizeof value. >> >> http://www.gossamer-threads.com/lists/engine?do=post_attachment;postatt_id=41157;list=linux > > Interestingly, many (all?) of the changes in that patch are wrong > because they don't try to match the terminating '\0'. As a result, > they will match against extensions of the target string as well as the > target string itself. > strNcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L270 memcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L651
On Mon, 6 Aug 2012, Pavel Vasilyev wrote: > >> http://www.gossamer-threads.com/lists/engine?do=post_attachment;postatt_id=41157;list=linux > > > > Interestingly, many (all?) of the changes in that patch are wrong > > because they don't try to match the terminating '\0'. As a result, > > they will match against extensions of the target string as well as the > > target string itself. > > > > strNcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L270 > memcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L651 Yes. So if s contains "abcde" then memcmp(s, "abc", 3) and strncmp(s, "abc", 3) will both return 0, and memcmp(s, "abc", 4) and strncmp(s, "abc", 4) will both return 1. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Silly question: when did sizeof("string") get changed to be anything
other than the size of the pointer ("string" is, after all, an array
of characters)?
> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org
> [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Alan Stern
> Sent: Monday, August 06, 2012 1:00 PM
> To: Pavel Vasilyev
> Cc: Pavel Machek; Len Brown; linux-acpi@vger.kernel.org;
> linux-pm@lists.linux-foundation.org;
> linux-kernel@vger.kernel.org; Len Brown
> Subject: Re: [linux-pm] [PATCH] ACPI: replace
> strlen("string") with sizeof("string") -1
>
> On Mon, 6 Aug 2012, Pavel Vasilyev wrote:
>
> > >>
> http://www.gossamer-threads.com/lists/engine?do=post_attachmen
> t;postatt_id=41157;list=linux
> > >
> > > Interestingly, many (all?) of the changes in that patch are wrong
> > > because they don't try to match the terminating '\0'. As
> a result,
> > > they will match against extensions of the target string
> as well as the
> > > target string itself.
> > >
> >
> > strNcmp compare N bytes -
> http://lxr.linux.no/#linux+v3.5/lib/string.c#L270
> > memcmp compare N bytes -
> http://lxr.linux.no/#linux+v3.5/lib/string.c#L651
>
> Yes. So if s contains "abcde" then
>
> memcmp(s, "abc", 3) and strncmp(s, "abc", 3) will both
> return 0, and
> memcmp(s, "abc", 4) and strncmp(s, "abc", 4) will both return 1.
>
> Alan Stern
>
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
> --
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
06.08.2012 23:59, Alan Stern ?????: > On Mon, 6 Aug 2012, Pavel Vasilyev wrote: > >>>> http://www.gossamer-threads.com/lists/engine?do=post_attachment;postatt_id=41157;list=linux >>> >>> Interestingly, many (all?) of the changes in that patch are wrong >>> because they don't try to match the terminating '\0'. As a result, >>> they will match against extensions of the target string as well as the >>> target string itself. >>> >> >> strNcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L270 >> memcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L651 > > Yes. So if s contains "abcde" then > > memcmp(s, "abc", 3) and strncmp(s, "abc", 3) will both return 0, and > memcmp(s, "abc", 4) and strncmp(s, "abc", 4) will both return 1. No matter what is contained in *s, "abcde" or "abcxxx", are important first N bytes. The second example, you see, a little bit stupid, and devoid of logic. :)
On Mon, 2012-08-06 at 22:57 +0000, Daniel Taylor wrote: > Silly question: when did sizeof("string") get changed to be anything > other than the size of the pointer ("string" is, after all, an array > of characters)? It is since K&R times that way. If you do not know the difference between a pointer and an array (and these are vastly different), go learn something new about C. Bernd
On Tue, 7 Aug 2012, Pavel Vasilyev wrote: > 06.08.2012 23:59, Alan Stern ?????: > > On Mon, 6 Aug 2012, Pavel Vasilyev wrote: > > > >>>> http://www.gossamer-threads.com/lists/engine?do=post_attachment;postatt_id=41157;list=linux > >>> > >>> Interestingly, many (all?) of the changes in that patch are wrong > >>> because they don't try to match the terminating '\0'. As a result, > >>> they will match against extensions of the target string as well as the > >>> target string itself. > >>> > >> > >> strNcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L270 > >> memcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L651 > > > > Yes. So if s contains "abcde" then > > > > memcmp(s, "abc", 3) and strncmp(s, "abc", 3) will both return 0, and > > memcmp(s, "abc", 4) and strncmp(s, "abc", 4) will both return 1. > > No matter what is contained in *s, "abcde" or "abcxxx", > are important first N bytes. The second example, you see, > a little bit stupid, and devoid of logic. :) Maybe yes, maybe no. It all depends on what you want. For example, if you're looking for "on" or "off", what should you do when the user writes "onoff"? You could accept it as meaning the same as "on", but if you were being careful then you would want to reject it as a meaningless value. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
07.08.2012 21:24, Alan Stern ?????: > On Tue, 7 Aug 2012, Pavel Vasilyev wrote: > >> 06.08.2012 23:59, Alan Stern ?????: >>> On Mon, 6 Aug 2012, Pavel Vasilyev wrote: >>> >>>>>> http://www.gossamer-threads.com/lists/engine?do=post_attachment;postatt_id=41157;list=linux >>>>> >>>>> Interestingly, many (all?) of the changes in that patch are wrong >>>>> because they don't try to match the terminating '\0'. As a result, >>>>> they will match against extensions of the target string as well as the >>>>> target string itself. >>>>> >>>> >>>> strNcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L270 >>>> memcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L651 >>> >>> Yes. So if s contains "abcde" then >>> >>> memcmp(s, "abc", 3) and strncmp(s, "abc", 3) will both return 0, and >>> memcmp(s, "abc", 4) and strncmp(s, "abc", 4) will both return 1. >> >> No matter what is contained in *s, "abcde" or "abcxxx", >> are important first N bytes. The second example, you see, >> a little bit stupid, and devoid of logic. :) > > Maybe yes, maybe no. It all depends on what you want. > > For example, if you're looking for "on" or "off", what should you do > when the user writes "onoff"? You could accept it as meaning the same > as "on", but if you were being careful then you would want to reject it > as a meaningless value. The users should't be allowed to think! There is "on" - the size of 2 bytes, or "off" - 3 bytes, other variations - user error. We do not create a kernel with artificial intelligence? ;)
Said it was a silly question.
It's funny.
I've been using "0123456789abcdef"[index] for a long time, so I "know"
that "string" is a array of char, but it never occurred to me that
"string" would work in sizeof() the same way as
char string[] = { '0', '1', '2', '3', '4', '5', '6', '7',
'8', '9', 'a', 'b', 'c', 'd', 'e', 'f', '\0' };
int stringlength = sizeof(string);
Learned something.
Thanks,
Dan
> -----Original Message-----
> From: Bernd Petrovitsch [mailto:bernd@petrovitsch.priv.at]
> Sent: Tuesday, August 07, 2012 6:20 AM
> To: Daniel Taylor
> Cc: 'Alan Stern'; Pavel Vasilyev; Pavel Machek; Len Brown;
> linux-acpi@vger.kernel.org;
> linux-pm@lists.linux-foundation.org;
> linux-kernel@vger.kernel.org; Len Brown
> Subject: RE: [linux-pm] [PATCH] ACPI: replace
> strlen("string") with sizeof("string") -1
>
> On Mon, 2012-08-06 at 22:57 +0000, Daniel Taylor wrote:
> > Silly question: when did sizeof("string") get changed to
> be anything
> > other than the size of the pointer ("string" is, after all, an array
> > of characters)?
>
> It is since K&R times that way.
> If you do not know the difference between a pointer and an array (and
> these are vastly different), go learn something new about C.
>
> Bernd
> --
> Bernd Petrovitsch Email : bernd@petrovitsch.priv.at
> LUGA : http://www.luga.at
>
> --
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 8 Aug 2012, Pavel Vasilyev wrote: > >>> Yes. So if s contains "abcde" then > >>> > >>> memcmp(s, "abc", 3) and strncmp(s, "abc", 3) will both return 0, and > >>> memcmp(s, "abc", 4) and strncmp(s, "abc", 4) will both return 1. > >> > >> No matter what is contained in *s, "abcde" or "abcxxx", > >> are important first N bytes. The second example, you see, > >> a little bit stupid, and devoid of logic. :) > > > > Maybe yes, maybe no. It all depends on what you want. > > > > For example, if you're looking for "on" or "off", what should you do > > when the user writes "onoff"? You could accept it as meaning the same > > as "on", but if you were being careful then you would want to reject it > > as a meaningless value. > > > The users should't be allowed to think! > There is "on" - the size of 2 bytes, or "off" - 3 bytes, > other variations - user error. > > We do not create a kernel with artificial intelligence? ;) Let me rephrase the previous statement, as it appears you did not understand what I meant. If the kernel is testing for "on" or "off", what should it do when the user writes "onoff"? The kernel could accept this as meaning the same as "on", but if the kernel was being careful then it should reject "onoff" as a meaningless value. A 2-byte comparison for "on" would accept "onoff" whereas a 3-byte comparison would not. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c index 240a244..7c3f98b 100644 --- a/drivers/acpi/sysfs.c +++ b/drivers/acpi/sysfs.c @@ -173,7 +173,7 @@ static int param_set_trace_state(const char *val, struct kernel_param *kp) { int result = 0; - if (!strncmp(val, "enable", strlen("enable"))) { + if (!strncmp(val, "enable", sizeof("enable") - 1)) { result = acpi_debug_trace(trace_method_name, trace_debug_level, trace_debug_layer, 0);
...both give the number of chars in the string without the '\0', as strncmp() wants, but sizeof() is compile-time. Reported-by: Alan Stern <stern@rowland.harvard.edu> Cc: Pavel Vasilyev <pavel@pavlinux.ru> Signed-off-by: Len Brown <len.brown@intel.com> --- drivers/acpi/sysfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) if (result) @@ -181,7 +181,7 @@ static int param_set_trace_state(const char *val, struct kernel_param *kp) goto exit; } - if (!strncmp(val, "disable", strlen("disable"))) { + if (!strncmp(val, "disable", sizeof("disable") - 1)) { int name = 0; result = acpi_debug_trace((char *)&name, trace_debug_level, trace_debug_layer, 0);