This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Pedro Alves wrote:
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.

Habit and symmetry. I'll change it.



- 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.


OK, test for zero added, how's this?





2011-02-22  Michael Snyder  <msnyder@vmware.com>

	* cli/cli-utils.c (number_is_in_list): Check for zero return.

Index: cli/cli-utils.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-utils.c,v
retrieving revision 1.9
diff -u -p -u -p -r1.9 cli-utils.c
--- cli/cli-utils.c	21 Feb 2011 23:40:46 -0000	1.9
+++ cli/cli-utils.c	22 Feb 2011 18:24:22 -0000
@@ -175,10 +175,15 @@ number_is_in_list (char *list, int numbe
   if (list == NULL || *list == '\0')
     return 1;
 
-  while (list != NULL && *list != '\0')
-    if (get_number_or_range (&list) == number)
-      return 1;
+  while (*list != '\0')
+    {
+      int gotnum = get_number_or_range (&list);
 
+      if (gotnum == 0)
+	error (_("Args must be numbers or '$' variables."));
+      if (gotnum == number)
+	return 1;
+    }
   return 0;
 }
 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]