From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 73872 invoked by alias); 21 Sep 2017 11:11: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 73858 invoked by uid 89); 21 Sep 2017 11:11:43 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=no version=3.3.2 spammy=guidelines, Hx-languages-length:1918, nearer, H*M:4a51 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 21 Sep 2017 11:11:42 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id ABA95129896; Thu, 21 Sep 2017 11:11:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com ABA95129896 Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves@redhat.com Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id CF8CE5C541; Thu, 21 Sep 2017 11:11:39 +0000 (UTC) Subject: Re: [RFA 49/67] Constify some commands in btrace.c To: "Metzger, Markus T" , Tom Tromey , "gdb-patches@sourceware.org" References: <20170921051023.19023-1-tom@tromey.com> <20170921051023.19023-50-tom@tromey.com> From: Pedro Alves Message-ID: <74f1184e-0d97-1082-4a51-3ae99c55717a@redhat.com> Date: Thu, 21 Sep 2017 11:11:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-09/txt/msg00617.txt.bz2 On 09/21/2017 08:02 AM, Metzger, Markus T wrote: >> @@ -3213,13 +3214,16 @@ get_context_size (char **arg) >> if (!isdigit (*pos)) >> error (_("Expected positive number, got: %s."), pos); >> >> - return strtol (pos, arg, 10); >> + char *end; >> + long result = strtol (pos, &end, 10); >> + *arg = end; >> + return result; >> } > > The rest of the declarations are at the beginning. I'd prefer to keep it that way. Interesting. Out of curiosity, is there a particular reason for that preference? I ask because we haven't adjusted our guidelines in this area yet, but I've been asking people to move declarations nearer where they're initialized in patch reviews. My reasoning for that has been that IMHO, declarations nearer to where they're initialized help with a few things: - helps with reasoning about the code, since the type of the variable has more changes of being visible. The top of the scope may be a good number of lines above. - helps prevent accidental used-uninitialized bugs. - avoid gratuitous pessimization when the variable is of non-trivial type, by avoiding separate default construction + assignment steps. (though that's not the case here, since in this case all variables are scalars). The point that I feel particularly strongly about is the last one, about non-trivial types, though as said that doesn't apply in this case. For the first two points, in C89, we'd some times open a extra scope, like in this case you could write: { char *end; long result = strtol (pos, &end, 10); *arg = end; return result; } In C99/C++, we can write declarations in the middle of blocks without forcing the extra scope. It's really up to you the style to use for this code as btrace maintainer, but I hope that at least this clarifies things for those that end up being asked different things from different people. :-) Thanks, Pedro Alves