cifs: use standard token parser for mount options
Subsystems (2)
Hello Sachin Prabhu,
This is a semi-automatic email about new static checker warnings.
The patch 8830d7e07a5e: "cifs: use standard token parser for mount options" from Mar 23, 2012, leads to the following Smatch complaint:
fs/cifs/connect.c:1607 cifs_parse_mount_options()
error: we previously assumed 'value' could be null (see line 1568)
fs/cifs/connect.c
1567 value = strchr(data, '=');
1568 if (value != NULL)
^^^^^^^^^^^^^
Check.
1569 *value++ = '\0';
1570
1571 /* Set tmp_end to end of the string */
1572 tmp_end = (char *) value + strlen(value);
^^^^^We can't run strlen() on NULL pointers.
1573
1574 /* Check if following character is the deliminator
regards,
dan carpenter
A message that is not in the archive (sent off-list or not yet received): [email protected]
Hello Dan,
You are right.
Quoted 7 lines · Dan Carpenter
Hello Sachin Prabhu,
This is a semi-automatic email about new static checker warnings.
The patch
8830d7e07a5e: "cifs: use standard token parser for mount options" from Mar 23, 2012, leads to the following Smatch complaint:
Quoted 8 lines
fs/cifs/connect.c:1607cifs_parse_mount_options()
error: we previously assumed 'value' could be null (see line 1568)
fs/cifs/connect.c
1567 value =strchr(data, '=');
1568 if (value != NULL)^^^^^^^^^^^^^Check.
static const match_table_t cifs_mount_option_tokens = { ..
{ Opt_pass, "pass=%s" },
{ Opt_pass, "password=%s" },..
^^^ The parser will only match Opt_pass if the token passed contains a '=' character so value cannot be NULL. That check for NULL is unnecessary and shouldn't be there. This is otherwise harmless.
This is an artifact of the old code which was backported. Opt_pass needed special handling to allow it to process password with ',' characters in them. This bit of code was at top of the while loop and was used to parse the values passed with the mount option. This was set to point to the value in case the mount option was passed with a value or set to NULL in case it was parsing mount options which didn't pass any values.
In this case, the Opt_pass will be called only when it contains a value. The case where a blank password was set is handled separately with Opt_blank_pass.
Quoted 6 lines
1569 *value++ = '\0';
1570
1571 /* Settmp_endto end of the string */1572 tmp_end = (char *) value + strlen(value); ^^^^^
Quoted 7 lines
We can't run
strlen()on NULL pointers.1573
1574 /* Check if following character is the deliminatorregards,
dan carpenter
Sachin Prabhu
Quoted 21 lines · Sachin Prabhu
Hello Dan,
You are right.
Quoted 7 lines · Dan Carpenter
Hello Sachin Prabhu,
This is a semi-automatic email about new static checker warnings.
The patch
8830d7e07a5e: "cifs: use standard token parser for mount options" from Mar 23, 2012, leads to the following Smatch complaint:Quoted 8 lines
fs/cifs/connect.c:1607cifs_parse_mount_options()
error: we previously assumed 'value' could be null (see line 1568)
fs/cifs/connect.c
1567 value =strchr(data, '=');
1568 if (value != NULL)^^^^^^^^^^^^^Check.
Quoted 9 lines
static const
match_table_tcifs_mount_option_tokens= { ..{ Opt_pass, "pass=%s" }, { Opt_pass, "password=%s" },..
^^^ The parser will only matchOpt_passif the token passed contains a '=' character so value cannot be NULL. That check for NULL is unnecessary and shouldn't be there. This is otherwise harmless.
Great. Could you send a patch to remove the unneeded check? Please give me the Reported-by: tag, I am trying to catch up to Randy.
regards,
dan carpenter
PATCH 1/1 Remove unnecessary check for NULL in password parser
The password parser has an unnecessary check for a NULL value which triggers warnings in source checking tools. The code contains artifacts from the old parsing code which are no longer required.
--- fs/cifs/connect.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
fs/cifs/connect.c +1 -2
@@ -1565,8 +1565,7 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,/* Obtain the value string */ value =strchr (data,'=' ); -if (value !=NULL ) - *value++ ='\0' ; + value++;/* Set tmp_end to end of the string */ tmp_end = (char *) value +strlen (value);
Signature · 1 line
1.7.7.6
Re: [PATCH] Remove unnecessary check for NULL in password parser
Quoted 14 lines · On Wed, 28 Mar 2012 18:07:08 +0100 Sachin Prabhu…
The password parser has an unnecessary check for a NULL value which triggers warnings in source checking tools. The code contains artifacts from the old parsing code which are no longer required.
Signed-off-by Sachin Prabhu <[email protected]>--- fs/cifs/connect.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)fs/cifs/connect.c +0
-0
fs/cifs/connect.c cifs_parse_mount_options(const char *mountdata, const char *devname, · line 1571
Quoted 9 lines
fs/cifs/connect.c +1
-2@@ -1565,8 +1565,7 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,/* Obtain the value string */ value =strchr (data,'=' ); -if (value !=NULL ) - *value++ ='\0' ; + value++;/* Set tmp_end to end of the string */ tmp_end = (char *) value +strlen (value);