Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 4/4] Fix PR gdb/15294: list with unlimited listsize broken
Date: Thu, 28 Mar 2013 09:12:00 -0000	[thread overview]
Message-ID: <20130327204425.24420.85758.stgit@brno.lan> (raw)
In-Reply-To: <20130327204357.24420.94722.stgit@brno.lan>

Currently, "set listsize -1" is supposed to mean "unlimited" source
lines, but, alas, it doesn't actually work:

 (gdb) set listsize -1
 (gdb) show listsize
 Number of source lines gdb will list by default is unlimited.
 (gdb) list 1
 (gdb) list 1
 (gdb) list 1
 (gdb) set listsize 10
 (gdb) list 1
 1       /* Main function for CLI gdb.
 2          Copyright (C) 2002-2013 Free Software Foundation, Inc.
 3
 4          This file is part of GDB.
 5
 6          This program is free software; you can redistribute it and/or modify
 7          it under the terms of the GNU General Public License as published by
 8          the Free Software Foundation; either version 3 of the License, or
 9          (at your option) any later version.
 10

Before this patch:

     http://sourceware.org/ml/gdb-patches/2012-08/msg00367.html

was applied, the "set listsize" command was a var_integer command, and
"unlimited" was set with 0.  Internally, var_integer maps 0 to INT_MAX

   case var_integer:
      {
      ...
	if (val == 0 && c->var_type == var_integer)
	  val = INT_MAX;

The change in that patch to zuinteger_unlimited command, meant that -1
is left as -1 in the command's control variable (lines_to_list), and
the code in source.c isn't expecting that -- it only expects positive
numbers.

I previously suggested fixing the code and keeping the new behavior,
but I found that "set listsize 0" is currently used in the wild, and
we do have a bunch of other commands where "0" means unlimited, so I'm
thinking that changing this command alone in isolation is not a good
idea.

So I now strongly prefer reverting back the behavior in 7.6 to the same
behavior the command has had since 2006 (0==unlimited, -1=error).
Before that, set listsize -1 would be accepted as unlimited as well.

After 7.6 is out, in mainline, we can get back to reconsidering
changing this command's behavior, if there's a real need for being
able to suppress output.  For now, let's play it safe.

The previous patches made it so that the list line 1 with unlimited
listsize actually passes now, so we can remove the xfail.

The list.exp test was originally written years and years ago expecting
0 mean "no output", but GDB never actually worked that way.  This
adjusts the test to the old behavior, so that the test actually
passes, and the xfail is removed.

gdb/
2013-03-27  Pedro Alves  <palves@redhat.com>

	PR gdb/15294

	* source.c (_initialize_source): Change back "set listsize" to an
	integer command.

gdb/testsuite/
2013-03-27  Pedro Alves  <palves@redhat.com>

	* gdb.base/list.exp (set_listsize): Adjust to accept $arg == 0 to
	mean unlimited instead of $arg < 0.
	(test_listsize): Remove "listsize of 0 suppresses output" test.
	Test that "set listsize 0" ends up with an unlimited listsize.
---
 gdb/source.c                    |    8 ++++----
 gdb/testsuite/gdb.base/list.exp |   10 ++--------
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/gdb/source.c b/gdb/source.c
index 2d9410e..03c5253 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -2044,12 +2044,12 @@ The matching line number is also stored as the value of \"$_\"."));
       add_com_alias ("?", "reverse-search", class_files, 0);
     }
 
-  add_setshow_zuinteger_unlimited_cmd ("listsize", class_support,
-				       &lines_to_list, _("\
+  add_setshow_integer_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, show_lines_to_list,
-				       &setlist, &showlist);
+			    NULL,
+			    show_lines_to_list,
+			    &setlist, &showlist);
 
   add_cmd ("substitute-path", class_files, set_substitute_path_command,
            _("\
diff --git a/gdb/testsuite/gdb.base/list.exp b/gdb/testsuite/gdb.base/list.exp
index 915b255..96ca198 100644
--- a/gdb/testsuite/gdb.base/list.exp
+++ b/gdb/testsuite/gdb.base/list.exp
@@ -67,7 +67,7 @@ proc set_listsize { arg } {
     if [gdb_test_no_output "set listsize $arg" "setting listsize to $arg #$set_listsize_count"] {
 	return 0
     }
-    if { $arg < 0 } {
+    if { $arg == 0 } {
 	set arg "unlimited";
     }
 
@@ -136,15 +136,9 @@ proc test_listsize {} {
 	gdb_test "list 10" "1\[ \t\]+#include \"list0.h\".*\r\n${last_line_re}" "list line 10 with listsize 100"
     }
 
-    # Try listsize of 0 which suppresses printing.
+    # Try listsize of 0 which is special, and means unlimited.
 
     set_listsize 0
-    gdb_test "list 1" "" "listsize of 0 suppresses output"
-
-    # Try listsize of -1 which is special, and means unlimited.
-
-    set_listsize -1
-    setup_xfail "*-*-*"
     gdb_test "list 1" "1\[ \t\]+#include .*\r\n${last_line_re}" "list line 1 with unlimited listsize"
 }
 


  parent reply	other threads:[~2013-03-27 20:44 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-28  6:28 [PATCH 0/4 v2] [mainline+7.6] " Pedro Alves
2013-03-28  1:11 ` [PATCH 1/4] list.exp: Catch "set listsize" failures (and "set listsize -1/0"'s history) Pedro Alves
2013-03-28  6:09 ` [PATCH 2/4] list.exp: Adjust "set listsize -1" to current test source's real line count Pedro Alves
2013-03-28  7:44 ` [PATCH 3/4] list.exp: Avoid hardcoding line numbers Pedro Alves
2013-03-28  9:12 ` Pedro Alves [this message]
2013-03-28  9:26   ` [PATCH 4/4] Fix PR gdb/15294: list with unlimited listsize broken Pedro Alves
2013-03-28 16:47     ` Pedro Alves
2013-03-28 15:39 ` [PATCH 0/4 v2] [mainline+7.6] " Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130327204425.24420.85758.stgit@brno.lan \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox