On Thu, 2015-02-26 at 14:16 +0300, Dan Carpenter wrote:
> Hello Jason Wessel,
>
> The patch 5d5314d6795f: "kdb: core for kgdb back end (1 of 2)" from
> May 20, 2010, leads to the following static checker warning:
>
> kernel/debug/kdb/kdb_io.c:352 kdb_read()
> warn: bool is not less than zero.
>
> kernel/debug/kdb/kdb_io.c
> 337 len = strlen(p_tmp);
> 338 count = kallsyms_symbol_complete(p_tmp,
> 339 sizeof(tmpbuffer) -
> 340 (p_tmp - tmpbuffer));
> 341 if (tab == 2 && count > 0) {
> 342 kdb_printf("\n%d symbols are found.", count);
> 343 if (count > dtab_count) {
> 344 count = dtab_count;
> 345 kdb_printf(" But only first %d
> symbols will"
> 346 " be printed.\nYou can
> change the"
> 347 " environment variable
> DTABCOUNT.",
> 348 count);
> 349 }
> 350 kdb_printf("\n");
> 351 for (i = 0; i < count; i++) {
> 352 if (kallsyms_symbol_next(p_tmp, i) <
> 0)
>
> The kallsyms_symbol_next() function returns 1 on found and 0 on not
> found so this test should be "== 0" instead of "< 0".
I had a look at this.
This code is defensive in nature; it is intended to trigger if
kallsyms_symbol_complete() returns a bad count. That, combined with the
way kallsyms_symbol_next() returns a boolean means I reckon the best fix
to be:
if (WARN_ON(!kallsyms_symbol_next(p_tmp, i))
I have this in patch form and will send it out after a few more tests.
> 353 break;
> 354 kdb_printf("%s ", p_tmp);
>
> Presumably we print the same thing over and over "count" times.
kallsyms_symbol_next() internally maintains a static iterator (the
second argument is used to reset-or-not the iterator) so it will not
yield the same result when called.
>
> 355 *(p_tmp + len) = '\0';
> ^^^
> "len" was calculated back on line 337. We don't need to update it? I'm
> not sure I understand this line.
Whilst kallsyms_symbol_next() maintains a static iterator to record the
position of the search within the symbol table it does *not* record the
search string that it is looking for. Line 355 shortens the search
string (which currently contains a search *result* rather than the
search target) back to its original value ready to do the next search.
Daniel.
>
> 356 }
>
> regards,
> dan carpenter
>
> ------------------------------------------------------------------------------
> Dive into the World of Parallel Programming The Go Parallel Website, sponsored
> by Intel and developed in partnership with Slashdot Media, is your hub for all
> things parallel software development, from weekly thought leadership blogs to
> news, videos, case studies, tutorials and more. Take a look and join the
> conversation now. http://goparallel.sourceforge.net/
> _______________________________________________
> Kgdb-bugreport mailing list
> Kgdb-bugreport@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport
------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
Kgdb-bugreport mailing list
Kgdb-bugreport@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport
