Re: [commit] fix for "info threads" printing multiple headers

[Pedro Alves <pedro@codesourcery.com>]

Thanks!  A few comments below.  If you don't want to
fix these, I'll try to find a bit later on myself.

On Monday 21 February 2011 23:40:06, Michael Snyder wrote:
> +int
> +number_is_in_list (char *list, int number)
> +{
> +  if (list == NULL || *list == '\0')
> +    return 1;
> +
> +  while (list != NULL && *list != '\0')
> +    if (get_number_or_range (&list) == number)
> +      return 1;
> +
> +  return 0;
> +}

- why the 'list != NULL' check in the while loop?
get_number_or_range never puts NULL in the output
argument, and NULL lists are short circuited above.

- get_number_or_range mantains an internal state machine
using static variables.  I think that as long as you
always pass in the same list string, and the list spec
string is correctly formed, you're not hitting stale
state inside get_number_or_range.  It'd be nicer
if get_number_or_range (or a variant which get_number_or_range
would then be implemented on top of) took an additional
struct pointer that pointed to a struct that held
all the currently static state.  This function would then
always pass in a pointer into such a newly initialized
object on the stack.

- get_number_or_range returns '0' on error.  You can
see a bunch of its old callers in breakpoint.c handling
that '0' case specially.  It comes from get_number_trailer:

	  /* Return zero, which caller must interpret as error.  */
	  retval = 0;

I suspect that we'll want to make than an "error" call or
to adjust the interface of these functions to allow
returning an error indication unambiguous with zero
the number, as soon as we want to reuse this function
in places where zero makes sense as valid number...

Examples of not handling the special zero:

(gdb) info threads a 1
  Id   Target Id         Frame 
* 1    Thread 0x7ffff7fd6700 (LWP 13769) "threads" 0x00007ffff7bc7285 in pthread_join () from /lib/libpthread.so.0
(gdb)

(gdb) info threads a-1
No threads match 'a-1'.

(gdb) info threads a -1
  Id   Target Id         Frame 
* 1    Thread 0x7ffff7fd6700 (LWP 13769) "threads" 0x00007ffff7bc7285 in pthread_join () from /lib/libpthread.so.0
(gdb) 

The last one only "works" by accident due to the extra space.

-- 
Pedro Alves