From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24276 invoked by alias); 24 Dec 2012 04:11:32 -0000 Received: (qmail 24238 invoked by uid 22791); 24 Dec 2012 04:11:30 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_HOSTKARMA_NO X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 24 Dec 2012 04:11:26 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id CAC8A2E442; Sun, 23 Dec 2012 23:11:25 -0500 (EST) 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 cx4PvDBKfk5I; Sun, 23 Dec 2012 23:11:25 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id CCD4E2E448; Sun, 23 Dec 2012 23:11:24 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 836A4C26BE; Mon, 24 Dec 2012 08:11:16 +0400 (RET) Date: Mon, 24 Dec 2012 04:11:00 -0000 From: Joel Brobecker To: Sergio Durigan Junior Cc: Eli Zaretskii , Pierre Muller , gdb-patches@sourceware.org Subject: Re: [RFA] (cli/cli-cmds.c) ARI fix: Avoid assignment inside if statement Message-ID: <20121224041116.GP5370@adacore.com> References: <001201cde13f$af3ad4b0$0db07e10$%muller@ics-cnrs.unistra.fr> <837go8pnxn.fsf@gnu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) 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: 2012-12/txt/msg00800.txt.bz2 > > Since when is that bad C, so much so that we would need to enforce it? > > Interesting... I remember when I started hacking GDB, I was strongly > discouraged to do assignments inside `if' checkings. I don't remember > who told me that, but the reason was something related to the Coding > Standards (I don't have any references either). Anyway, since that > moment I stopped doing this... It is in the GNU Coding Standards: http://www.gnu.org/prep/standards/standards.html#Syntactic-Conventions In particular: | Try to avoid assignments inside if-conditions (assignments inside | while-conditions are ok). For example, don’t write this: | | if ((foo = (char *) malloc (sizeof *foo)) == 0) | fatal ("virtual memory exhausted"); | instead, write this: | | foo = (char *) malloc (sizeof *foo); | if (foo == 0) | fatal ("virtual memory exhausted"); FWIW, I mostly agree with this suggestion, particulary in the examples above. I do remember seeing some examples where it was more practical if we could have assignments inside conditions (in an "if ... else if ..." situation), but even then, I find that it makes it harder to notice the assignment. In the examples that Pierre chose to fix, it seems obvious to me (this is a judgement call, so potentially subject to disagreement) that there was no need to have the assignment inside the condition, and that the patched code is easier to grasp. I think it is a clear improvement. > FWIW, I agree that it is not bad C, and totally valid I think that the point is not that this is bad C, or not valid/portable. I think that the point is that it is judged to be a poor practice. -- Joel