From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 84688 invoked by alias); 21 Mar 2015 08:26:50 -0000 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 Received: (qmail 84678 invoked by uid 89); 21 Mar 2015 08:26:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_SOFTFAIL autolearn=no version=3.3.2 X-HELO: mtaout27.012.net.il Received: from mtaout27.012.net.il (HELO mtaout27.012.net.il) (80.179.55.183) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 21 Mar 2015 08:26:47 +0000 Received: from conversion-daemon.mtaout27.012.net.il by mtaout27.012.net.il (HyperSendmail v2007.08) id <0NLJ00B00YZR7000@mtaout27.012.net.il> for gdb-patches@sourceware.org; Sat, 21 Mar 2015 10:21:27 +0200 (IST) Received: from HOME-C4E4A596F7 ([87.69.4.28]) by mtaout27.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0NLJ00AR8Z7Q1110@mtaout27.012.net.il>; Sat, 21 Mar 2015 10:21:27 +0200 (IST) Date: Sat, 21 Mar 2015 08:26:00 -0000 From: Eli Zaretskii Subject: Re: [PATCH 3/4] GDBServer: introduce --server-stderr command line option In-reply-to: <1426905265-8495-4-git-send-email-crosa@redhat.com> To: Cleber Rosa Cc: gdb-patches@sourceware.org, crosa@redhat.com, areis@redhat.com Reply-to: Eli Zaretskii Message-id: <83384yvjr0.fsf@gnu.org> References: <1426905265-8495-1-git-send-email-crosa@redhat.com> <1426905265-8495-4-git-send-email-crosa@redhat.com> X-IsSubscribed: yes X-SW-Source: 2015-03/txt/msg00686.txt.bz2 > From: Cleber Rosa > Cc: crosa@redhat.com, areis@redhat.com > Date: Fri, 20 Mar 2015 23:34:24 -0300 > > This command line option will redirect all of the gdbserver's own > output (always sent to stderr) to a separate file. This feature > makes it possible to distinguish between the inferior process stderr > and gdbserver's own stderr. > [...] > +@cindex @option{--server-stderr}, @code{gdbserver} option > +The @option{--server-stderr} option tells @code{gdbserver} that any content > +that it would normally generate itself to its own @code{stderr} should be > +redirected to another file. This is useful if you want to keep the > +inferior @code{stderr} separate from the one generated by @code{gdbserver}. Note how the text description of the rationale for the change you posted to explain it to us is much more useful than the technical description in the manual. Why not say in the manual what you told us? >From the user POV, the fact that gdbserver uses stderr for its own output is an implementation detail that is almost unimportant. (It could be important for redirection purposes, but the command-line option you introduce eliminates the need to redirect in most cases, right?) What _is_ important is that gdbserver's own output will be redirected to that file, and that important information gets lost in the confusing "it would normally generate itself to its own 'stderr'", which leaves unanswered the question what part of gdbserver's output is "normally" excluded from that. For this reason, I suggest to name the option "--server-output". And I think we want a NEWS entry for this change. > +@item --server-stderr > +Instruct @code{gdbserver} to redirect its own @code{stderr} to another > +file. The option requires an argument, so the argument should be mentioned with the option and referenced in the text that describes it. > +static int > +set_server_stderr (char *path) > +{ > + FILE *tmp; > + > + tmp = fopen (path, "w"); > + if (tmp == NULL) > + { > + fprintf (stderr, > + "Could not open server stderr file '%s': %s\n", > + path, strerror(errno)); > + return -1; > + } > + server_stderr = tmp; > + return 0; > +} > + > void > monitor_show_help (void) > { > @@ -3017,6 +3036,7 @@ gdbserver_usage (FILE *stream) > " none\n" > " timestamp\n" > " --remote-debug Enable remote protocol debugging output.\n" > + " --server-stderr=PATH Redirect server's STDERR to file at PATH.\n" > " --version Display version information and exit.\n" > " --wrapper WRAPPER -- Run WRAPPER to start new programs.\n" > " --once Exit after the first connection has " > @@ -3186,7 +3206,13 @@ captured_main (int argc, char *argv[]) > > while (*next_arg != NULL && **next_arg == '-') > { > - if (strcmp (*next_arg, "--version") == 0) > + if (strncmp (*next_arg, "--server-stderr=", > + sizeof ("--server-stderr=") - 1) == 0) > + { > + char *path = *next_arg + (sizeof ("--server-stderr=") - 1); > + set_server_stderr (path); > + } > + else if (strcmp (*next_arg, "--version") == 0) AFAIK, GNU Coding Standards frown on using "path" for anything that is not PATH-style list of directories. So please use "file" or "file name" here. Thanks.