From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24820 invoked by alias); 4 Jan 2008 12:20:02 -0000 Received: (qmail 24788 invoked by uid 22791); 4 Jan 2008 12:20:00 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 04 Jan 2008 12:19:37 +0000 Received: (qmail 13896 invoked from network); 4 Jan 2008 12:19:33 -0000 Received: from unknown (HELO ?78.130.84.60?) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 4 Jan 2008 12:19:33 -0000 Message-ID: <477E23F9.7040904@codesourcery.com> Date: Fri, 04 Jan 2008 12:20:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.13pre) Gecko/20071023 Thunderbird/1.5.0.14pre Mnenhy/0.7.5.0 MIME-Version: 1.0 To: Joel Brobecker CC: gdb-patches@sourceware.org Subject: Re: --pid and --core References: <477D122A.4060405@codesourcery.com> <20080104045813.GA28411@adacore.com> In-Reply-To: <20080104045813.GA28411@adacore.com> Content-Type: multipart/mixed; boundary="------------040106020204070507090705" Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-01/txt/msg00053.txt.bz2 This is a multi-part message in MIME format. --------------040106020204070507090705 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1362 Joel Brobecker wrote: > Thanks for the patch. A couple of of comments: > Thanks for taking a look! > It took me a long while to understand this loop. I realize this was > already implemented that way before, but I think something like the code > below is simpler. What do you think? > I completely agree. It looks much cleaner. Thanks. > if (argc - optind > 0) I just changed these to use the (optind < argc) idiom, which is already used in the file. > /* Any argument left on the command line is unexpected and will be > ignored. Inform the user. */ > for (; optind < argc; optind++) > fprintf_unfiltered (...); > I'm keeping the output format (ARG) or (ARG ...) if (optind < argc). There's a wrinkle. The 'int count' variable that was originally used in the loop, was also used to do a last resort stack alignment in captured_main's entry. #if defined (ALIGN_STACK_ON_STARTUP) i = (int) &count & 0x03; if (i != 0) alloca (4 - i); #endif ALIGN_STACK_ON_STARTUP isn't defined in any target/host anymore, and if it was, count is in the middle of the stack frame for long and nobody complained. We can safely remove it. It's mentioned in the gdbint manual, so that'll go too. Now with correct email in the ChangeLog entry ... Retested on i686-pc-linux-gnu. OK ? Eli, gdbint patch OK ? -- Pedro Alves --------------040106020204070507090705 Content-Type: text/x-diff; name="pid_core_split.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="pid_core_split.diff" Content-length: 6022 2008-01-03 Pedro Alves gdb/ * main.c (captured_main): Remove 'count' varible and the ALIGN_STACK_ON_ENTRY block that used it. Error out if --core and --pid options were issued simultaneously. If an explicit pid option was passed, don't fallback to core file. Detect extra arguments better in the presence of explicit pid or core arguments. 2008-01-03 Pedro Alves gdb/doc/ * gdbint.texinfo (Host Conditionals): Remove mention of ALIGN_STACK_ON_ENTRY. --- gdb/doc/gdbint.texinfo | 7 --- gdb/main.c | 88 +++++++++++++++++++++++++++---------------------- 2 files changed, 50 insertions(+), 45 deletions(-) Index: src/gdb/main.c =================================================================== --- src.orig/gdb/main.c 2008-01-02 17:58:39.000000000 +0000 +++ src/gdb/main.c 2008-01-04 11:36:05.000000000 +0000 @@ -119,7 +119,6 @@ captured_main (void *data) struct captured_main_args *context = data; int argc = context->argc; char **argv = context->argv; - int count; static int quiet = 0; static int batch = 0; static int set_args = 0; @@ -127,7 +126,9 @@ captured_main (void *data) /* Pointers to various arguments from command line. */ char *symarg = NULL; char *execarg = NULL; + char *pidarg = NULL; char *corearg = NULL; + char *pid_or_core_arg = NULL; char *cdarg = NULL; char *ttyarg = NULL; @@ -175,12 +176,6 @@ captured_main (void *data) lim_at_start = (char *) sbrk (0); #endif -#if defined (ALIGN_STACK_ON_STARTUP) - i = (int) &count & 0x3; - if (i != 0) - alloca (4 - i); -#endif - cmdsize = 1; cmdarg = (struct cmdarg *) xmalloc (cmdsize * sizeof (*cmdarg)); ncmd = 0; @@ -435,8 +430,7 @@ captured_main (void *data) corearg = optarg; break; case 'p': - /* "corearg" is shared by "--core" and "--pid" */ - corearg = optarg; + pidarg = optarg; break; case 'x': cmdarg[ncmd].type = CMDARG_FILE; @@ -572,26 +566,34 @@ extern int gdbtk_test (char *); } else { - /* OK, that's all the options. The other arguments are filenames. */ - count = 0; - for (; optind < argc; optind++) - switch (++count) - { - case 1: - symarg = argv[optind]; - execarg = argv[optind]; - break; - case 2: - /* The documentation says this can be a "ProcID" as well. - We will try it as both a corefile and a pid. */ - corearg = argv[optind]; - break; - case 3: - fprintf_unfiltered (gdb_stderr, - _("Excess command line arguments ignored. (%s%s)\n"), - argv[optind], (optind == argc - 1) ? "" : " ..."); - break; - } + /* OK, that's all the options. */ + + /* The first argument, if specified, is the name of the + executable. */ + if (optind < argc) + { + symarg = argv[optind]; + execarg = argv[optind]; + optind++; + } + + /* If the user hasn't already specified a PID or the name of a + core file, then a second optional argument is allowed. If + present, this argument should be interpreted as either a + PID or a core file, whichever works. */ + if (pidarg == NULL && corearg == NULL && optind < argc) + { + pid_or_core_arg = argv[optind]; + optind++; + } + + /* Any argument left on the command line is unexpected and + will be ignored. Inform the user. */ + if (optind < argc) + fprintf_unfiltered (gdb_stderr, _("\ +Excess command line arguments ignored. (%s%s)\n"), + argv[optind], + (optind == argc - 1) ? "" : " ..."); } if (batch) quiet = 1; @@ -733,21 +735,31 @@ extern int gdbtk_test (char *); catch_command_errors (symbol_file_add_main, symarg, 0, RETURN_MASK_ALL); } + if (corearg && pidarg) + error (_("\ +Can't attach to process and specify a core file at the same time.")); + if (corearg != NULL) - { - /* corearg may be either a corefile or a pid. - If its first character is a digit, try attach first - and then corefile. Otherwise try corefile first. */ + catch_command_errors (core_file_command, corearg, + !batch, RETURN_MASK_ALL); + else if (pidarg != NULL) + catch_command_errors (attach_command, pidarg, + !batch, RETURN_MASK_ALL); + else if (pid_or_core_arg) + { + /* The user specified 'gdb program pid' or gdb program core'. + If pid_or_core_arg's first character is a digit, try attach + first and then corefile. Otherwise try just corefile. */ - if (isdigit (corearg[0])) + if (isdigit (pid_or_core_arg[0])) { - if (catch_command_errors (attach_command, corearg, + if (catch_command_errors (attach_command, pid_or_core_arg, !batch, RETURN_MASK_ALL) == 0) - catch_command_errors (core_file_command, corearg, + catch_command_errors (core_file_command, pid_or_core_arg, !batch, RETURN_MASK_ALL); } - else /* Can't be a pid, better be a corefile. */ - catch_command_errors (core_file_command, corearg, + else /* Can't be a pid, better be a corefile. */ + catch_command_errors (core_file_command, pid_or_core_arg, !batch, RETURN_MASK_ALL); } Index: src/gdb/doc/gdbint.texinfo =================================================================== --- src.orig/gdb/doc/gdbint.texinfo 2008-01-04 11:35:30.000000000 +0000 +++ src/gdb/doc/gdbint.texinfo 2008-01-04 11:35:51.000000000 +0000 @@ -2490,13 +2490,6 @@ of a function to be called if @code{SIGW Define this to expand into code that will define the function named by the expansion of @code{SIGWINCH_HANDLER}. -@item ALIGN_STACK_ON_STARTUP -@cindex stack alignment -Define this if your system is of a sort that will crash in -@code{tgetent} if the stack happens not to be longword-aligned when -@code{main} is called. This is a rare situation, but is known to occur -on several different types of systems. - @item CRLF_SOURCE_FILES @cindex DOS text files Define this if host files use @code{\r\n} rather than @code{\n} as a --------------040106020204070507090705--