From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9201 invoked by alias); 31 Jan 2006 11:32:17 -0000 Received: (qmail 9191 invoked by uid 22791); 31 Jan 2006 11:32:15 -0000 X-Spam-Check-By: sourceware.org Received: from server7.nfra.nl (HELO server7.nfra.nl) (192.87.1.57) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 31 Jan 2006 11:32:13 +0000 Received: from jop31.nfra.nl [10.87.10.31] by server7.nfra.nl; Tue, 31 Jan 2006 12:31:46 +0100 Received: from jop31.nfra.nl (localhost [127.0.0.1]) by jop31.nfra.nl (8.13.1/8.12.7/SuSE Linux 0.6) with ESMTP id k0VBVjD2022309 for ; Tue, 31 Jan 2006 12:31:46 +0100 Received: (from kettenis@localhost) by jop31.nfra.nl (8.13.1/8.13.1/Submit) id k0VBVjf8022306; Tue, 31 Jan 2006 12:31:45 +0100 Date: Tue, 31 Jan 2006 11:32:00 -0000 Message-Id: <200601311131.k0VBVjf8022306@jop31.nfra.nl> From: Mark Kettenis To: gdb-patches@sourceware.org Subject: [RFC] Clean up var_integer/var_uinteger/var_zinteger mess Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-01/txt/msg00488.txt.bz2 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 * 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")); }