From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27903 invoked by alias); 22 May 2014 17:22:44 -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 27889 invoked by uid 89); 22 May 2014 17:22:43 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.2 X-HELO: usevmg20.ericsson.net Received: from usevmg20.ericsson.net (HELO usevmg20.ericsson.net) (198.24.6.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Thu, 22 May 2014 17:22:42 +0000 Received: from EUSAAHC006.ericsson.se (Unknown_Domain [147.117.188.90]) by usevmg20.ericsson.net (Symantec Mail Security) with SMTP id A0.4E.27529.592ED735; Thu, 22 May 2014 13:42:13 +0200 (CEST) Received: from [142.133.183.104] (147.117.188.8) by smtps-am.internal.ericsson.com (147.117.188.90) with Microsoft SMTP Server (TLS) id 14.3.174.1; Thu, 22 May 2014 13:22:37 -0400 Message-ID: <537E325D.4050908@ericsson.com> Date: Thu, 22 May 2014 17:22:00 -0000 From: Simon Marchi User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-Version: 1.0 To: Joel Brobecker CC: Subject: Re: [PATCH] Add comment for mi_run_cmd_full References: <1400708905-14184-1-git-send-email-simon.marchi@ericsson.com> <20140521222611.GQ22822@adacore.com> In-Reply-To: <20140521222611.GQ22822@adacore.com> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2014-05/txt/msg00560.txt.bz2 On 14-05-21 06:26 PM, Joel Brobecker wrote: > Hi Simon, > >> 2014-05-21 Simon Marchi >> >> * lib/mi-support.exp (mi_run_cmd_full): Add comments. > > Overall, this looks good to me. I wasn't aware of the history, > so just did a little bit of catching up :-), but I confirm that > the description you propose matches the actual behavior... > >> +# use_mi_command selects whether MI or CLI commands are used by the >> +# procedure. >> +# args is passed to the command used to run the test program, "run" or >> +# "-exec-run", depending on the value of use_mi_command. Therefore, if >> +# use_mi_command is false, they are effectively args to the test program. >> +# If use_mi_command is true, they are simply arguments to -exec-run. >> proc mi_run_cmd_full {use_mi_command args} { >> global suppress_flag >> if { $suppress_flag } { >> -- >> 2.0.0.rc0 > > When speaking of the value of a variable, the variable name should be > in all caps. For instance: > > # USE_MI_COMMAND selects whether MI or CLI commands are used by the > # procedure. Ok. > If I may suggest a different approach (anyone is welcome to comment > on that too), it'd be useful to succintly say what the function does > first, before getting into the details of the parameters. Also, I would > group the semantics of each value of use_mi_command together. So, > one example: > > # Send the command to run the test program. > # > # If USE_MI_COMMAND is true, the "-exec-run" command is used. > # Otherwise, the "run" (CLI) command is used. > # > # ARGS is passed as argument to the command used to run the test program. > # Beware that arguments to "-exec-run" do not have the same semantics as > # arguments to the "run" command, so USE_MI_COMMAND influences the meaning > # of ARGS. If USE_MI_COMMAND is true, they are arguments to -exec-run. > # If USE_MI_COMMAND is false, they are effectively arguments passed > # to the test program. > > I hope it helps! > I think this is very clear. Should I post an updated patch ?