[PATCH 4/4] fs/qnx4: decrement sizeof size in strncmp

[PATCH 4/4] fs/qnx4: decrement sizeof size in strncmp

[Julia Lawall]

From: Julia Lawall <julia@diku.dk>

As observed by Joe Perches, sizeof of a constant string includes the
trailing 0.  If what is wanted is to check the initial characters of
another string, this trailing 0 should not be taken into account.  If an
exact match is wanted, strcmp should be used instead.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression foo;
constant char *abc;
@@

strncmp(foo, abc, 
- sizeof(abc)
+ sizeof(abc)-1
 )
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 fs/qnx4/inode.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/fs/qnx4/inode.c b/fs/qnx4/inode.c
index 449f5a6..0614b00 100644
--- a/fs/qnx4/inode.c
+++ b/fs/qnx4/inode.c
@@ -221,7 +221,9 @@ static const char *qnx4_checkroot(struct super_block *sb)
 				rootdir = (struct qnx4_inode_entry *) (bh->b_data + i * QNX4_DIR_ENTRY_SIZE);
 				if (rootdir->di_fname != NULL) {
 					QNX4DEBUG((KERN_INFO "rootdir entry found : [%s]\n", rootdir->di_fname));
-					if (!strncmp(rootdir->di_fname, QNX4_BMNAME, sizeof QNX4_BMNAME)) {
+					if (!strncmp(rootdir->di_fname,
+						     QNX4_BMNAME,
+						     sizeof QNX4_BMNAME - 1)) {
 						found = 1;
 						qnx4_sb(sb)->BitMap = kmalloc( sizeof( struct qnx4_inode_entry ), GFP_KERNEL );
 						if (!qnx4_sb(sb)->BitMap) {

[PATCH 4/4] fs/qnx4: decrement sizeof size in strncmp

[Julia Lawall]

From: Julia Lawall <julia@diku.dk>

As observed by Joe Perches, sizeof of a constant string includes the
trailing 0.  If what is wanted is to check the initial characters of
another string, this trailing 0 should not be taken into account.  If an
exact match is wanted, strcmp should be used instead.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression foo;
constant char *abc;
@@

strncmp(foo, abc, 
- sizeof(abc)
+ sizeof(abc)-1
 )
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 fs/qnx4/inode.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/fs/qnx4/inode.c b/fs/qnx4/inode.c
index 449f5a6..0614b00 100644
--- a/fs/qnx4/inode.c
+++ b/fs/qnx4/inode.c
@@ -221,7 +221,9 @@ static const char *qnx4_checkroot(struct super_block *sb)
 				rootdir = (struct qnx4_inode_entry *) (bh->b_data + i * QNX4_DIR_ENTRY_SIZE);
 				if (rootdir->di_fname != NULL) {
 					QNX4DEBUG((KERN_INFO "rootdir entry found : [%s]\n", rootdir->di_fname));
-					if (!strncmp(rootdir->di_fname, QNX4_BMNAME, sizeof QNX4_BMNAME)) {
+					if (!strncmp(rootdir->di_fname,
+						     QNX4_BMNAME,
+						     sizeof QNX4_BMNAME - 1)) {
 						found = 1;
 						qnx4_sb(sb)->BitMap = kmalloc( sizeof( struct qnx4_inode_entry ), GFP_KERNEL );
 						if (!qnx4_sb(sb)->BitMap) {

Re: [PATCH 4/4] fs/qnx4: decrement sizeof size in strncmp

[Anders Larsen]

On 2009-11-12 08:49:44, Julia Lawall wrote:
> As observed by Joe Perches, sizeof of a constant string includes the
> trailing 0.  If what is wanted is to check the initial characters of
> another string, this trailing 0 should not be taken into account.  If an
> exact match is wanted, strcmp should be used instead.

Good catch.
However, in this case an exact match (with the string ".bitmap") is what
is needed, so

NAK

Will you cook up a new patch changing strncmp to strcmp?

Cheers
Anders

Re: [PATCH 4/4] fs/qnx4: decrement sizeof size in strncmp

[Anders Larsen]

On 2009-11-12 08:49:44, Julia Lawall wrote:
> As observed by Joe Perches, sizeof of a constant string includes the
> trailing 0.  If what is wanted is to check the initial characters of
> another string, this trailing 0 should not be taken into account.  If an
> exact match is wanted, strcmp should be used instead.

Good catch.
However, in this case an exact match (with the string ".bitmap") is what
is needed, so

NAK

Will you cook up a new patch changing strncmp to strcmp?

Cheers
Anders

Re: [PATCH 4/4] fs/qnx4: decrement sizeof size in strncmp

[Julia Lawall]

On Thu, 12 Nov 2009, Anders Larsen wrote:

> On 2009-11-12 08:49:44, Julia Lawall wrote:
> > As observed by Joe Perches, sizeof of a constant string includes the
> > trailing 0.  If what is wanted is to check the initial characters of
> > another string, this trailing 0 should not be taken into account.  If an
> > exact match is wanted, strcmp should be used instead.
> 
> Good catch.
> However, in this case an exact match (with the string ".bitmap") is what
> is needed, so
> 
> NAK
> 
> Will you cook up a new patch changing strncmp to strcmp?

Sure.

julia

Re: [PATCH 4/4] fs/qnx4: decrement sizeof size in strncmp

[Julia Lawall]

On Thu, 12 Nov 2009, Anders Larsen wrote:

> On 2009-11-12 08:49:44, Julia Lawall wrote:
> > As observed by Joe Perches, sizeof of a constant string includes the
> > trailing 0.  If what is wanted is to check the initial characters of
> > another string, this trailing 0 should not be taken into account.  If an
> > exact match is wanted, strcmp should be used instead.
> 
> Good catch.
> However, in this case an exact match (with the string ".bitmap") is what
> is needed, so
> 
> NAK
> 
> Will you cook up a new patch changing strncmp to strcmp?

Sure.

julia

Re: [PATCH 4/4] fs/qnx4: decrement sizeof size in strncmp

[Julia Lawall]

From: Julia Lawall <julia@diku.dk>

As an identical match is wanted in this case, strcmp can be used instead.

The semantic match that lead to detecting this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression foo;
constant char *abc;
@@

*strncmp(foo, abc, sizeof(abc))
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 fs/qnx4/inode.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/qnx4/inode.c b/fs/qnx4/inode.c
index 449f5a6..150f4af 100644
--- a/fs/qnx4/inode.c
+++ b/fs/qnx4/inode.c
@@ -221,7 +221,8 @@ static const char *qnx4_checkroot(struct super_block *sb)
 				rootdir = (struct qnx4_inode_entry *) (bh->b_data + i * QNX4_DIR_ENTRY_SIZE);
 				if (rootdir->di_fname != NULL) {
 					QNX4DEBUG((KERN_INFO "rootdir entry found : [%s]\n", rootdir->di_fname));
-					if (!strncmp(rootdir->di_fname, QNX4_BMNAME, sizeof QNX4_BMNAME)) {
+					if (!strcmp(rootdir->di_fname,
+						    QNX4_BMNAME)) {
 						found = 1;
 						qnx4_sb(sb)->BitMap = kmalloc( sizeof( struct qnx4_inode_entry ), GFP_KERNEL );
 						if (!qnx4_sb(sb)->BitMap) {

Re: [PATCH 4/4] fs/qnx4: decrement sizeof size in strncmp

[Julia Lawall]

From: Julia Lawall <julia@diku.dk>

As an identical match is wanted in this case, strcmp can be used instead.

The semantic match that lead to detecting this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression foo;
constant char *abc;
@@

*strncmp(foo, abc, sizeof(abc))
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 fs/qnx4/inode.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/qnx4/inode.c b/fs/qnx4/inode.c
index 449f5a6..150f4af 100644
--- a/fs/qnx4/inode.c
+++ b/fs/qnx4/inode.c
@@ -221,7 +221,8 @@ static const char *qnx4_checkroot(struct super_block *sb)
 				rootdir = (struct qnx4_inode_entry *) (bh->b_data + i * QNX4_DIR_ENTRY_SIZE);
 				if (rootdir->di_fname != NULL) {
 					QNX4DEBUG((KERN_INFO "rootdir entry found : [%s]\n", rootdir->di_fname));
-					if (!strncmp(rootdir->di_fname, QNX4_BMNAME, sizeof QNX4_BMNAME)) {
+					if (!strcmp(rootdir->di_fname,
+						    QNX4_BMNAME)) {
 						found = 1;
 						qnx4_sb(sb)->BitMap = kmalloc( sizeof( struct qnx4_inode_entry ), GFP_KERNEL );
 						if (!qnx4_sb(sb)->BitMap) {

sizeof vs strlen (was Re: [PATCH 4/4] fs/qnx4: decrement sizeof

[Bernd Petrovitsch]

On Thu, 2009-11-12 at 08:49 +0100, Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
> 
> As observed by Joe Perches, sizeof of a constant string includes the
> trailing 0.  If what is wanted is to check the initial characters of
> another string, this trailing 0 should not be taken into account.  If an
> exact match is wanted, strcmp should be used instead.
[...]
> strncmp(foo, abc, 
> - sizeof(abc)
> + sizeof(abc)-1
>  )
> // </smpl>
Am I the only one who find "strlen()" instead of "sizeof()-1" here much
more readable (and to the point).

As for run-time, it shouldn't make a difference for static/constant
strings as gcc should be able calculate the length at compile time. And
if the string is not constant, sizeof() is probably wrong anyways.

	Bernd
-- 
Firmix Software GmbH                   http://www.firmix.at/
mobil: +43 664 4416156                 fax: +43 1 7890849-55
          Embedded Linux Development and Services

sizeof vs strlen (was Re: [PATCH 4/4] fs/qnx4: decrement sizeof size in strncmp)

[Bernd Petrovitsch]

On Thu, 2009-11-12 at 08:49 +0100, Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
> 
> As observed by Joe Perches, sizeof of a constant string includes the
> trailing 0.  If what is wanted is to check the initial characters of
> another string, this trailing 0 should not be taken into account.  If an
> exact match is wanted, strcmp should be used instead.
[...]
> strncmp(foo, abc, 
> - sizeof(abc)
> + sizeof(abc)-1
>  )
> // </smpl>
Am I the only one who find "strlen()" instead of "sizeof()-1" here much
more readable (and to the point).

As for run-time, it shouldn't make a difference for static/constant
strings as gcc should be able calculate the length at compile time. And
if the string is not constant, sizeof() is probably wrong anyways.

	Bernd
-- 
Firmix Software GmbH                   http://www.firmix.at/
mobil: +43 664 4416156                 fax: +43 1 7890849-55
          Embedded Linux Development and Services

Re: sizeof vs strlen (was Re: [PATCH 4/4] fs/qnx4: decrement sizeof

[Julia Lawall]

On Thu, 12 Nov 2009, Bernd Petrovitsch wrote:

> On Thu, 2009-11-12 at 08:49 +0100, Julia Lawall wrote:
> > From: Julia Lawall <julia@diku.dk>
> > 
> > As observed by Joe Perches, sizeof of a constant string includes the
> > trailing 0.  If what is wanted is to check the initial characters of
> > another string, this trailing 0 should not be taken into account.  If an
> > exact match is wanted, strcmp should be used instead.
> [...]
> > strncmp(foo, abc, 
> > - sizeof(abc)
> > + sizeof(abc)-1
> >  )
> > // </smpl>
> Am I the only one who find "strlen()" instead of "sizeof()-1" here much
> more readable (and to the point).
> 
> As for run-time, it shouldn't make a difference for static/constant
> strings as gcc should be able calculate the length at compile time. And
> if the string is not constant, sizeof() is probably wrong anyways.

The rule specifies that the string is a constant.

julia

Re: sizeof vs strlen (was Re: [PATCH 4/4] fs/qnx4: decrement sizeof size in strncmp)

[Julia Lawall]

On Thu, 12 Nov 2009, Bernd Petrovitsch wrote:

> On Thu, 2009-11-12 at 08:49 +0100, Julia Lawall wrote:
> > From: Julia Lawall <julia@diku.dk>
> > 
> > As observed by Joe Perches, sizeof of a constant string includes the
> > trailing 0.  If what is wanted is to check the initial characters of
> > another string, this trailing 0 should not be taken into account.  If an
> > exact match is wanted, strcmp should be used instead.
> [...]
> > strncmp(foo, abc, 
> > - sizeof(abc)
> > + sizeof(abc)-1
> >  )
> > // </smpl>
> Am I the only one who find "strlen()" instead of "sizeof()-1" here much
> more readable (and to the point).
> 
> As for run-time, it shouldn't make a difference for static/constant
> strings as gcc should be able calculate the length at compile time. And
> if the string is not constant, sizeof() is probably wrong anyways.

The rule specifies that the string is a constant.

julia

Re: sizeof vs strlen (was Re: [PATCH 4/4] fs/qnx4: decrement sizeof

[Julia Lawall]

On Thu, 12 Nov 2009, Bernd Petrovitsch wrote:

> On Thu, 2009-11-12 at 08:49 +0100, Julia Lawall wrote:
> > From: Julia Lawall <julia@diku.dk>
> > 
> > As observed by Joe Perches, sizeof of a constant string includes the
> > trailing 0.  If what is wanted is to check the initial characters of
> > another string, this trailing 0 should not be taken into account.  If an
> > exact match is wanted, strcmp should be used instead.
> [...]
> > strncmp(foo, abc, 
> > - sizeof(abc)
> > + sizeof(abc)-1
> >  )
> > // </smpl>
> Am I the only one who find "strlen()" instead of "sizeof()-1" here much
> more readable (and to the point).
> 
> As for run-time, it shouldn't make a difference for static/constant
> strings as gcc should be able calculate the length at compile time. And
> if the string is not constant, sizeof() is probably wrong anyways.

Does gcc have access to the definition of strlen?  It does not seem to be 
an inlined function, eg in lib/string.c.

julia

Re: sizeof vs strlen (was Re: [PATCH 4/4] fs/qnx4: decrement sizeof size in strncmp)

[Julia Lawall]

On Thu, 12 Nov 2009, Bernd Petrovitsch wrote:

> On Thu, 2009-11-12 at 08:49 +0100, Julia Lawall wrote:
> > From: Julia Lawall <julia@diku.dk>
> > 
> > As observed by Joe Perches, sizeof of a constant string includes the
> > trailing 0.  If what is wanted is to check the initial characters of
> > another string, this trailing 0 should not be taken into account.  If an
> > exact match is wanted, strcmp should be used instead.
> [...]
> > strncmp(foo, abc, 
> > - sizeof(abc)
> > + sizeof(abc)-1
> >  )
> > // </smpl>
> Am I the only one who find "strlen()" instead of "sizeof()-1" here much
> more readable (and to the point).
> 
> As for run-time, it shouldn't make a difference for static/constant
> strings as gcc should be able calculate the length at compile time. And
> if the string is not constant, sizeof() is probably wrong anyways.

Does gcc have access to the definition of strlen?  It does not seem to be 
an inlined function, eg in lib/string.c.

julia

Re: sizeof vs strlen (was Re: [PATCH 4/4] fs/qnx4: decrement

[Bernd Petrovitsch]

On Thu, 2009-11-12 at 16:33 +0100, Julia Lawall wrote: 
> On Thu, 12 Nov 2009, Bernd Petrovitsch wrote:
> 
> > On Thu, 2009-11-12 at 08:49 +0100, Julia Lawall wrote:
> > > From: Julia Lawall <julia@diku.dk>
> > > 
> > > As observed by Joe Perches, sizeof of a constant string includes the
> > > trailing 0.  If what is wanted is to check the initial characters of
> > > another string, this trailing 0 should not be taken into account.  If an
> > > exact match is wanted, strcmp should be used instead.
> > [...]
> > > strncmp(foo, abc, 
> > > - sizeof(abc)
> > > + sizeof(abc)-1
> > >  )
> > > // </smpl>
> > Am I the only one who find "strlen()" instead of "sizeof()-1" here much
> > more readable (and to the point).
> > 
> > As for run-time, it shouldn't make a difference for static/constant
> > strings as gcc should be able calculate the length at compile time. And
> > if the string is not constant, sizeof() is probably wrong anyways.
> 
> Does gcc have access to the definition of strlen?  It does not seem to be 
> an inlined function, eg in lib/string.c.
Since "strlen()" is defined in the C-Standard C-compilers could rely on
the defined behaviour (but I don't know exactly how gcc behaves with
-ffreestanding for all supported versions).
Then there is __builin_strlen() (see also
http://gcc.gnu.org/onlinedocs/gcc-4.4.2/gcc/Other-Builtins.html#Other-Builtins).

Stepping a quite small abstraction layer higher:
include/linux/string.h has at teh end:
----  snip  ----
static inline bool strstarts(const char *str, const char *prefix)
{
	return strncmp(str, prefix, strlen(prefix)) = 0;
}
---- snip  ----
seems to be what most uses of strnmcp() actually are: check if one
string is a prefix of another.

	Bernd
-- 
Firmix Software GmbH                   http://www.firmix.at/
mobil: +43 664 4416156                 fax: +43 1 7890849-55
          Embedded Linux Development and Services

Re: sizeof vs strlen (was Re: [PATCH 4/4] fs/qnx4: decrement sizeof size in strncmp)

[Bernd Petrovitsch]

On Thu, 2009-11-12 at 16:33 +0100, Julia Lawall wrote: 
> On Thu, 12 Nov 2009, Bernd Petrovitsch wrote:
> 
> > On Thu, 2009-11-12 at 08:49 +0100, Julia Lawall wrote:
> > > From: Julia Lawall <julia@diku.dk>
> > > 
> > > As observed by Joe Perches, sizeof of a constant string includes the
> > > trailing 0.  If what is wanted is to check the initial characters of
> > > another string, this trailing 0 should not be taken into account.  If an
> > > exact match is wanted, strcmp should be used instead.
> > [...]
> > > strncmp(foo, abc, 
> > > - sizeof(abc)
> > > + sizeof(abc)-1
> > >  )
> > > // </smpl>
> > Am I the only one who find "strlen()" instead of "sizeof()-1" here much
> > more readable (and to the point).
> > 
> > As for run-time, it shouldn't make a difference for static/constant
> > strings as gcc should be able calculate the length at compile time. And
> > if the string is not constant, sizeof() is probably wrong anyways.
> 
> Does gcc have access to the definition of strlen?  It does not seem to be 
> an inlined function, eg in lib/string.c.
Since "strlen()" is defined in the C-Standard C-compilers could rely on
the defined behaviour (but I don't know exactly how gcc behaves with
-ffreestanding for all supported versions).
Then there is __builin_strlen() (see also
http://gcc.gnu.org/onlinedocs/gcc-4.4.2/gcc/Other-Builtins.html#Other-Builtins).

Stepping a quite small abstraction layer higher:
include/linux/string.h has at teh end:
----  snip  ----
static inline bool strstarts(const char *str, const char *prefix)
{
	return strncmp(str, prefix, strlen(prefix)) == 0;
}
---- snip  ----
seems to be what most uses of strnmcp() actually are: check if one
string is a prefix of another.

	Bernd
-- 
Firmix Software GmbH                   http://www.firmix.at/
mobil: +43 664 4416156                 fax: +43 1 7890849-55
          Embedded Linux Development and Services

Re: [PATCH 4/4] fs/qnx4: decrement sizeof size in strncmp

[Anders Larsen]

From: Julia Lawall <julia@diku.dk>

As an identical match is wanted in this case, strcmp can be used instead.

The semantic match that lead to detecting this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression foo;
constant char *abc;
@@

*strncmp(foo, abc, sizeof(abc))
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>
Signed-off-by: Anders Larsen <al@alarsen.net>

---
 fs/qnx4/inode.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/qnx4/inode.c b/fs/qnx4/inode.c
index 449f5a6..150f4af 100644
--- a/fs/qnx4/inode.c
+++ b/fs/qnx4/inode.c
@@ -221,7 +221,8 @@ static const char *qnx4_checkroot(struct super_block *sb)
 				rootdir = (struct qnx4_inode_entry *) (bh->b_data + i * QNX4_DIR_ENTRY_SIZE);
 				if (rootdir->di_fname != NULL) {
 					QNX4DEBUG((KERN_INFO "rootdir entry found : [%s]\n", rootdir->di_fname));
-					if (!strncmp(rootdir->di_fname, QNX4_BMNAME, sizeof QNX4_BMNAME)) {
+					if (!strcmp(rootdir->di_fname,
+						    QNX4_BMNAME)) {
 						found = 1;
 						qnx4_sb(sb)->BitMap = kmalloc( sizeof( struct qnx4_inode_entry ), GFP_KERNEL );
 						if (!qnx4_sb(sb)->BitMap) {

Re: [PATCH 4/4] fs/qnx4: decrement sizeof size in strncmp

[Anders Larsen]

From: Julia Lawall <julia@diku.dk>

As an identical match is wanted in this case, strcmp can be used instead.

The semantic match that lead to detecting this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression foo;
constant char *abc;
@@

*strncmp(foo, abc, sizeof(abc))
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>
Signed-off-by: Anders Larsen <al@alarsen.net>

---
 fs/qnx4/inode.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/qnx4/inode.c b/fs/qnx4/inode.c
index 449f5a6..150f4af 100644
--- a/fs/qnx4/inode.c
+++ b/fs/qnx4/inode.c
@@ -221,7 +221,8 @@ static const char *qnx4_checkroot(struct super_block *sb)
 				rootdir = (struct qnx4_inode_entry *) (bh->b_data + i * QNX4_DIR_ENTRY_SIZE);
 				if (rootdir->di_fname != NULL) {
 					QNX4DEBUG((KERN_INFO "rootdir entry found : [%s]\n", rootdir->di_fname));
-					if (!strncmp(rootdir->di_fname, QNX4_BMNAME, sizeof QNX4_BMNAME)) {
+					if (!strcmp(rootdir->di_fname,
+						    QNX4_BMNAME)) {
 						found = 1;
 						qnx4_sb(sb)->BitMap = kmalloc( sizeof( struct qnx4_inode_entry ), GFP_KERNEL );
 						if (!qnx4_sb(sb)->BitMap) {

Re: [PATCH 4/4] fs/qnx4: decrement sizeof size in strncmp

[Jiri Kosina]

On Thu, 12 Nov 2009, Anders Larsen wrote:

> From: Julia Lawall <julia@diku.dk>
> 
> As an identical match is wanted in this case, strcmp can be used instead.
> 
> The semantic match that lead to detecting this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> expression foo;
> constant char *abc;
> @@
> 
> *strncmp(foo, abc, sizeof(abc))
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> Signed-off-by: Anders Larsen <al@alarsen.net>
> 
> ---
>  fs/qnx4/inode.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/qnx4/inode.c b/fs/qnx4/inode.c
> index 449f5a6..150f4af 100644
> --- a/fs/qnx4/inode.c
> +++ b/fs/qnx4/inode.c
> @@ -221,7 +221,8 @@ static const char *qnx4_checkroot(struct super_block *sb)
>  				rootdir = (struct qnx4_inode_entry *) (bh->b_data + i * QNX4_DIR_ENTRY_SIZE);
>  				if (rootdir->di_fname != NULL) {
>  					QNX4DEBUG((KERN_INFO "rootdir entry found : [%s]\n", rootdir->di_fname));
> -					if (!strncmp(rootdir->di_fname, QNX4_BMNAME, sizeof QNX4_BMNAME)) {
> +					if (!strcmp(rootdir->di_fname,
> +						    QNX4_BMNAME)) {
>  						found = 1;
>  						qnx4_sb(sb)->BitMap = kmalloc( sizeof( struct qnx4_inode_entry ), GFP_KERNEL );
>  						if (!qnx4_sb(sb)->BitMap) {

Doesn't seem to be present in 2.6.33-rc1.

Applied to my queue, thanks.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 4/4] fs/qnx4: decrement sizeof size in strncmp

[Jiri Kosina]

On Thu, 12 Nov 2009, Anders Larsen wrote:

> From: Julia Lawall <julia@diku.dk>
> 
> As an identical match is wanted in this case, strcmp can be used instead.
> 
> The semantic match that lead to detecting this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> expression foo;
> constant char *abc;
> @@
> 
> *strncmp(foo, abc, sizeof(abc))
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> Signed-off-by: Anders Larsen <al@alarsen.net>
> 
> ---
>  fs/qnx4/inode.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/qnx4/inode.c b/fs/qnx4/inode.c
> index 449f5a6..150f4af 100644
> --- a/fs/qnx4/inode.c
> +++ b/fs/qnx4/inode.c
> @@ -221,7 +221,8 @@ static const char *qnx4_checkroot(struct super_block *sb)
>  				rootdir = (struct qnx4_inode_entry *) (bh->b_data + i * QNX4_DIR_ENTRY_SIZE);
>  				if (rootdir->di_fname != NULL) {
>  					QNX4DEBUG((KERN_INFO "rootdir entry found : [%s]\n", rootdir->di_fname));
> -					if (!strncmp(rootdir->di_fname, QNX4_BMNAME, sizeof QNX4_BMNAME)) {
> +					if (!strcmp(rootdir->di_fname,
> +						    QNX4_BMNAME)) {
>  						found = 1;
>  						qnx4_sb(sb)->BitMap = kmalloc( sizeof( struct qnx4_inode_entry ), GFP_KERNEL );
>  						if (!qnx4_sb(sb)->BitMap) {

Doesn't seem to be present in 2.6.33-rc1.

Applied to my queue, thanks.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 4/4] fs/qnx4: decrement sizeof size in strncmp

[David Wagner]

Julia Lawall  wrote:
> As observed by Joe Perches, sizeof of a constant string includes the
> trailing 0.  If what is wanted is to check the initial characters of
> another string, this trailing 0 should not be taken into account.  If an
> exact match is wanted, strcmp should be used instead.
>
> The semantic patch that makes this change is as follows:

A caution: Your patch changes behavior.  Is there a specific reason
to believe that the change in behavior is what is desired/intended in
this context?  Lacking any analysis that indicates that the change in
behavior is desired, I'm skeptical that a behavior-changing patch should
be applied.

Re: [PATCH 4/4] fs/qnx4: decrement sizeof size in strncmp

[Joe Perches]

On Thu, 2009-11-12 at 23:49 +0000, David Wagner wrote:
> Julia Lawall  wrote:
> > As observed by Joe Perches, sizeof of a constant string includes the
> > trailing 0.  If what is wanted is to check the initial characters of
> > another string, this trailing 0 should not be taken into account.  If an
> > exact match is wanted, strcmp should be used instead.
> A caution: Your patch changes behavior.  Is there a specific reason
> to believe that the change in behavior is what is desired/intended in
> this context?  Lacking any analysis that indicates that the change in
> behavior is desired, I'm skeptical that a behavior-changing patch should
> be applied.

I agree that desired behavior should be known before
patches are applied.

Most likely, this instance should just be strcmp.

That gives better readability, trivially improved
performance, and easier verification.