[RFC] Clean up var_integer/var_uinteger/var_zinteger mess

[Mark Kettenis <kettenis at jive dot nl>]

While looking into tdep/2074, I noticed how crappy the code is that
handles var_integer/var_uinteger/var_zinteger.  I mean, var_zinteger
is supposed to be an 'int' but like Unsigned Integer?  var_integer is
supposed to be a (signed) integer, but when we set it, we use an
`unsigned int'.  The patch below cleans things up, but the source.c
change is debatable.

Unfortunately it also reveals a problem.  I now get:

FAIL: gdb.base/list.exp: show listsize unlimited #6
FAIL: gdb.base/list.exp: show listsize unlimited #7

Analyzing these tests shows that people have been truly confused,
because part of the test considers all values <= 0 to mean unlimited,
but other parts seem to consider 0 to mean that printing should be
suppressed.  Unfortunately the test that checks this is broken and
will always pass.  That may be why we didn't notice the incosistency
of the test before :(.

To me, the intended behaviour seems to be that 0 would suppress output
(0 lines printed), and -1 would print all lines (unlimited lines will
be printed).  But that doesn't map into any of the
var_integer/var_uinteger/var_zinteger cases.

Does anyone see a way out of this mess?  It seems that -1 meaning
unlimited never really worked.  So perhaps we should just drop those
tests and go with my patch.

Mark


Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* command.h (enum var_types): Reorder, adjust comments.
	* cli/cli-setshow.c (do_setshow_command): Handle var_integer,
	var_uinteger and var_zinteger according to their documented
	behaviour.
	* source.c (_initialize_source): Use add_setshow_zinteger_cmd for
	listsize.

Index: command.h
===================================================================
RCS file: /cvs/src/src/gdb/command.h,v
retrieving revision 1.54
diff -u -p -r1.54 command.h
--- command.h 17 Dec 2005 22:33:59 -0000 1.54
+++ command.h 31 Jan 2006 11:10:36 -0000
@@ -1,7 +1,7 @@
 /* Header file for command-reading library command.c.
 
    Copyright (C) 1986, 1989, 1990, 1991, 1992, 1993, 1994, 1995, 1999,
-   2000, 2002, 2004 Free Software Foundation, Inc.
+   2000, 2002, 2004, 2006 Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -54,24 +54,26 @@ cmd_types;
 /* Types of "set" or "show" command.  */
 typedef enum var_types
   {
-    /* "on" or "off".  *VAR is an integer which is nonzero for on,
-       zero for off.  */
+    /* "on" or "off".  *VAR is an int which is nonzero for on, zero
+       for off.  */
     var_boolean,
 
     /* "on" / "true" / "enable" or "off" / "false" / "disable" or
-       "auto.  *VAR is an ``enum auto_boolean''.  NOTE: In general a
+       "auto.  *VAR is an `enum auto_boolean'.  NOTE: In general a
        custom show command will need to be implemented - one that for
        "auto" prints both the "auto" and the current auto-selected
        value. */
     var_auto_boolean,
 
-    /* Unsigned Integer.  *VAR is an unsigned int.  The user can type 0
-       to mean "unlimited", which is stored in *VAR as UINT_MAX.  */
-    var_uinteger,
-
-    /* Like var_uinteger but signed.  *VAR is an int.  The user can type 0
-       to mean "unlimited", which is stored in *VAR as INT_MAX.  */
+    /* Signed integer.  *VAR is an int.  The user can type 0 to mean
+       "unlimited", which is stored in *VAR as INT_MAX.  */
     var_integer,
+    /* Unsigned integer.  *VAR is an unsigned int.  The user can type
+       0 to mean "unlimited", which is stored in *VAR as UINT_MAX.  */
+    var_uinteger,
+    /* Zeroable integer.  *VAR is an int.  Like var_integer except
+       that zero really means zero.  */
+    var_zinteger,
 
     /* String which the user enters with escapes (e.g. the user types \n and
        it is a real newline in the stored string).
@@ -80,17 +82,14 @@ typedef enum var_types
     /* String which stores what the user types verbatim.
        *VAR is a malloc'd string, or NULL if the string is empty.  */
     var_string_noescape,
-    /* String which stores a filename.  (*VAR) is a malloc'd string,
+    /* String which stores a filename.  *VAR is a malloc'd string.  */
+    var_filename,
+    /* String which stores a filename.  *VAR is a malloc'd string,
        or "" if the string was empty.  */
     var_optional_filename,
-    /* String which stores a filename.  (*VAR) is a malloc'd
-       string.  */
-    var_filename,
-    /* ZeroableInteger.  *VAR is an int.  Like Unsigned Integer except
-       that zero really means zero.  */
-    var_zinteger,
-    /* Enumerated type.  Can only have one of the specified values.  *VAR is a
-       char pointer to the name of the element that we find.  */
+
+    /* Enumerated type.  Can only have one of the specified values.  *VAR
+       is a char pointer to the name of the element that we find.  */
     var_enum
   }
 var_types;
Index: source.c
===================================================================
RCS file: /cvs/src/src/gdb/source.c,v
retrieving revision 1.72
diff -u -p -r1.72 source.c
--- source.c 15 Jan 2006 19:09:30 -0000 1.72
+++ source.c 31 Jan 2006 11:10:37 -0000
@@ -1638,7 +1638,7 @@ The matching line number is also stored 
       add_com_alias ("?", "reverse-search", class_files, 0);
     }
 
-  add_setshow_integer_cmd ("listsize", class_support, &lines_to_list, _("\
+  add_setshow_zinteger_cmd ("listsize", class_support, &lines_to_list, _("\
 Set number of source lines gdb will list by default."), _("\
 Show number of source lines gdb will list by default."), NULL,
 			    NULL,
Index: cli/cli-setshow.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-setshow.c,v
retrieving revision 1.27
diff -u -p -r1.27 cli-setshow.c
--- cli/cli-setshow.c 17 Dec 2005 22:40:17 -0000 1.27
+++ cli/cli-setshow.c 31 Jan 2006 11:10:37 -0000
@@ -200,31 +200,53 @@ do_setshow_command (char *arg, int from_
 	case var_auto_boolean:
 	  *(enum auto_boolean *) c->var = parse_auto_binary_operation (arg);
 	  break;
-	case var_uinteger:
-	  if (arg == NULL)
-	    error_no_arg (_("integer to set it to."));
-	  *(unsigned int *) c->var = parse_and_eval_long (arg);
-	  if (*(unsigned int *) c->var == 0)
-	    *(unsigned int *) c->var = UINT_MAX;
-	  break;
 	case var_integer:
 	  {
-	    unsigned int val;
+	    LONGEST val;
+
 	    if (arg == NULL)
 	      error_no_arg (_("integer to set it to."));
 	    val = parse_and_eval_long (arg);
+	    if (val < INT_MIN || val > INT_MAX)
+	      error (_("integer %s out of range"),
+		     int_string (val, 10, 1, 0, 0));
+
 	    if (val == 0)
 	      *(int *) c->var = INT_MAX;
-	    else if (val >= INT_MAX)
-	      error (_("integer %u out of range"), val);
 	    else
 	      *(int *) c->var = val;
-	    break;
 	  }
+	  break;
+	case var_uinteger:
+	  {
+	    LONGEST val;
+
+	    if (arg == NULL)
+	      error_no_arg (_("integer to set it to."));
+	    val = parse_and_eval_long (arg);
+	    if (val < 0 || val > UINT_MAX)
+	      error (_("integer %s out of range"),
+		     int_string (val, 10, 1, 0, 0));
+
+	    if (val == 0)
+	      *(unsigned int *) c->var = UINT_MAX;
+	    else
+	      *(unsigned int *) c->var = val;
+	  }
+	  break;
 	case var_zinteger:
-	  if (arg == NULL)
-	    error_no_arg (_("integer to set it to."));
-	  *(int *) c->var = parse_and_eval_long (arg);
+	  {
+	    LONGEST val;
+
+	    if (arg == NULL)
+	      error_no_arg (_("integer to set it to."));
+	    val = parse_and_eval_long (arg);
+	    if (val < INT_MIN || val > INT_MAX)
+	      error (_("integer %s out of range"),
+		     int_string (val, 10, 1, 0, 0));
+
+	    *(int *) c->var = val;
+	  }
 	  break;
 	case var_enum:
 	  {
@@ -332,25 +354,21 @@ do_setshow_command (char *arg, int from_
 	      break;
 	    }
 	  break;
-	case var_uinteger:
-	  if (*(unsigned int *) c->var == UINT_MAX)
-	    {
-	      fputs_filtered ("unlimited", stb->stream);
-	      break;
-	    }
-	  /* else fall through */
-	case var_zinteger:
-	  fprintf_filtered (stb->stream, "%u", *(unsigned int *) c->var);
-	  break;
 	case var_integer:
 	  if (*(int *) c->var == INT_MAX)
-	    {
-	      fputs_filtered ("unlimited", stb->stream);
-	    }
+	    fputs_filtered ("unlimited", stb->stream);
 	  else
 	    fprintf_filtered (stb->stream, "%d", *(int *) c->var);
 	  break;
-
+	case var_uinteger:
+	  if (*(unsigned int *) c->var == UINT_MAX)
+	    fputs_filtered ("unlimited", stb->stream);
+	  else
+	    fprintf_filtered (stb->stream, "%u", *(unsigned int *) c->var);
+	  break;
+	case var_zinteger:
+	  fprintf_filtered (stb->stream, "%d", *(int *) c->var);
+	  break;
 	default:
 	  error (_("gdb internal error: bad var_type in do_setshow_command"));
 	}