Hello Dan,
You are right.
On Wed, 2012-03-28 at 13:45 +0300, Dan Carpenter wrote:
> 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.
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.
>
> 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
Sachin Prabhu
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
