ACPI & Power Management Patches for Linux 3.5-rc4

From: Pavel Vasilyev <[email protected]>

Current code is ignoring the last character of "enable" and "disable" in comparisons.

https://bugzilla.kernel.org/show_bug.cgi?id=33732

Signed-off-by Len Brown <[email protected]>

---
 drivers/acpi/sysfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/acpi/sysfs.c +0 -0

index 9f66181..240a244 100644

...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.

...both give the number of chars in the string
without the '\0', as strncmp() wants,
but sizeof() is compile-time.

Quoted 3 lines · Len Brown

...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)) ...

?

Review

Quoted 2 lines

(quoted diff) +1 -1

-	if (!strncmp(val, "enable", strlen("enable"))) {
+	if (!strncmp(val, "enable", sizeof("enable") - 1)) {

Quoted 11 lines · Len Brown

Quoted 3 lines

...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)) ...

Quoted 4 lines

?

Review

Quoted 2 lines

(quoted diff) +1 -1

-	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.

06.08.2012 18:36, Alan Stern пишет:

Quoted 14 lines · Pavel Machek

Quoted 13 lines · Len Brown

Quoted 3 lines

...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)) ...

?

Quoted 6 lines

Quoted 2 lines

Review

Quoted 2 lines

(quoted diff) +1 -1

-	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

Quoted 23 lines

06.08.2012 18:36, Alan Stern пишет:

Quoted 20 lines · Pavel Machek

Quoted 9 lines · Len Brown

Quoted 3 lines

...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....

Quoted 6 lines

#define streq(a, b) ... if (_buildin_constant(b)) ...

?

Review

Quoted 2 lines

(quoted diff) +1 -1

-	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.

Quoted 1 line

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.

Quoted 7 lines

Quoted 1 line

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

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Alan Stern
Sent: Monday, August 06, 2012 1:00 PM
To: Pavel Vasilyev
Cc: Pavel Machek; Len Brown; [email protected]; [email protected]; [email protected]; Len Brown
Subject: Re: [linux-pm] [PATCH] ACPI: replace strlen("string") with sizeof("string") -1

Quoted 1 line · Pavel Vasilyev

Quoted 1 line

Quoted 1 line

http://www.gossamer-threads.com/lists/engine?do=post_attachmen
t;postatt_id=41157;list=linux

Quoted 3 lines

Quoted 3 lines

Interestingly, many (all?) of the changes in that patch are wrong because they don't try to match the terminating '\0'. As

a result,

Quoted 1 line

Quoted 1 line

they will match against extensions of the target string

as well as the

Quoted 4 lines

Quoted 2 lines

target string itself.

strNcmp compare N bytes -

http://lxr.linux.no/#linux+v3.5/lib/string.c#L270

Quoted 1 line

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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

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: Bernd Petrovitsch [mailto:[email protected]]
Sent: Tuesday, August 07, 2012 6:20 AM
To: Daniel Taylor
Cc: 'Alan Stern'; Pavel Vasilyev; Pavel Machek; Len Brown; [email protected]; [email protected]; [email protected]; Len Brown
Subject: RE: [linux-pm] [PATCH] ACPI: replace strlen("string") with sizeof("string") -1

Quoted 1 line · Daniel Taylor

Silly question: when did sizeof("string") get changed to

be anything

Quoted 2 lines

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

Signature · 2 lines

Bernd Petrovitsch                  Email : [email protected]
                     LUGA : http://www.luga.at

Quoted 9 lines

Quoted 8 lines

Quoted 2 lines

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.

Quoted 2 lines

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.

06.08.2012 23:59, Alan Stern пишет:

Quoted 13 lines · Pavel Vasilyev

Quoted 11 lines

Quoted 8 lines

Quoted 2 lines

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

Quoted 4 lines

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. :)

Quoted 15 lines

06.08.2012 23:59, Alan Stern пишет:

Quoted 13 lines · Pavel Vasilyev

Quoted 11 lines

Quoted 8 lines

Quoted 2 lines

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

Quoted 8 lines

Quoted 4 lines

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.

Quoted 9 lines

Quoted 8 lines

Quoted 4 lines

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. :)

Quoted 6 lines

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? ;)

...both give the number of chars in the string
without the '\0', as strncmp() wants,
but sizeof() is compile-time.

From: Zhang Rui <[email protected]>

Dell Precision M6600 is known to require PCI reboot, so add it to the reboot blacklist in pci_reboot_dmi_table[].

https://bugzilla.kernel.org/show_bug.cgi?id=42749

cc [email protected]

Signed-off-by Zhang Rui <[email protected]>

Signed-off-by Len Brown <[email protected]>

---
 arch/x86/kernel/reboot.c | 8 ++++++++
 1 file changed, 8 insertions(+)