From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31676 invoked by alias); 21 May 2014 22:26:15 -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 31665 invoked by uid 89); 21 May 2014 22:26:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Wed, 21 May 2014 22:26:13 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id AE36A11617E; Wed, 21 May 2014 18:26:11 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id lcsRPA8tzV2K; Wed, 21 May 2014 18:26:11 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 85A2A11617B; Wed, 21 May 2014 18:26:11 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 36EDE436C4; Wed, 21 May 2014 15:26:11 -0700 (PDT) Date: Wed, 21 May 2014 22:26:00 -0000 From: Joel Brobecker To: Simon Marchi Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] Add comment for mi_run_cmd_full Message-ID: <20140521222611.GQ22822@adacore.com> References: <1400708905-14184-1-git-send-email-simon.marchi@ericsson.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1400708905-14184-1-git-send-email-simon.marchi@ericsson.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2014-05/txt/msg00525.txt.bz2 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. 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! -- Joel