On Sun, 2014-12-14 at 23:52 +0100, Rickard Strandqvist wrote: > There is otherwise a risk of a possible null pointer dereference. > > Was largely found by using a static code analysis program called cppcheck. Perhaps the tool could use a little work. It's not possible for end to be NULL no? unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int base) { unsigned long long result; unsigned int rv; cp = _parse_integer_fixup_radix(cp, &base); rv = _parse_integer(cp, base, &result); /* FIXME */ cp += (rv & ~KSTRTOX_OVERFLOW); if (endp) *endp = (char *)cp; return result; } EXPORT_SYMBOL(simple_strtoull); > diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c [] Above this: whole = simple_strtoull(pbuf, &end, 10); > +++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c > @@ -1897,17 +1897,19 @@ int lprocfs_write_frac_u64_helper(const char *buffer, unsigned long count, > } > > units = 1; > - switch (*end) { > - case 'p': case 'P': > - units <<= 10; > - case 't': case 'T': > - units <<= 10; > - case 'g': case 'G': > - units <<= 10; > - case 'm': case 'M': > - units <<= 10; > - case 'k': case 'K': > - units <<= 10; > + if (end) { > + switch (*end) { > + case 'p': case 'P': > + units <<= 10; > + case 't': case 'T': > + units <<= 10; > + case 'g': case 'G': > + units <<= 10; > + case 'm': case 'M': > + units <<= 10; > + case 'k': case 'K': > + units <<= 10; > + } The only thing I might do is switch (tolower(*end)) { and remove the second case entry for each line _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel