From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5013 invoked by alias); 15 Mar 2011 09:19:17 -0000 Received: (qmail 4985 invoked by uid 22791); 15 Mar 2011 09:19:15 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,TW_BG X-Spam-Check-By: sourceware.org Received: from mail-iy0-f175.google.com (HELO mail-iy0-f175.google.com) (209.85.210.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 15 Mar 2011 09:19:10 +0000 Received: by iyb26 with SMTP id 26so402896iyb.20 for ; Tue, 15 Mar 2011 02:19:08 -0700 (PDT) MIME-Version: 1.0 Received: by 10.42.158.132 with SMTP id h4mr1869471icx.202.1300180748462; Tue, 15 Mar 2011 02:19:08 -0700 (PDT) Received: by 10.231.168.69 with HTTP; Tue, 15 Mar 2011 02:19:08 -0700 (PDT) In-Reply-To: <877hc9pkhp.fsf_-_@rho.meyering.net> References: <87zkp9zmq0.fsf@rho.meyering.net> <877hc9r8w6.fsf_-_@rho.meyering.net> <877hc9pkhp.fsf_-_@rho.meyering.net> Date: Tue, 15 Mar 2011 09:19:00 -0000 Message-ID: Subject: Re: [PATCH v3] Re: avoid useless if-before-free tests From: Janne Blomqvist To: Jim Meyering Cc: "Joseph S. Myers" , gcc-patches@gcc.gnu.org, java-patches@gcc.gnu.org, fortran@gcc.gnu.org Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Mailing-List: contact java-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: java-patches-owner@gcc.gnu.org X-SW-Source: 2011-q1/txt/msg00085.txt.bz2 On Tue, Mar 8, 2011 at 19:53, Jim Meyering wrote: > Relative to v2, I've added libgo/ to the list of exempt directories and a= dded > this recently discussed gfc_free patch, at the request of Tobias Burnus. > Also, I corrected an error in fortran's ChangeLog and removed all > whitespace changes from all ChangeLog files. The libgfortran changes are Ok for 4.7. For the gfortran frontend (gcc/fortran/*) I'd prefer if you'd - Replace all calls to "gfc_free (x)" with "free (x)". - Remove the gfc_free() function and prototype. - Remove the free() macro which currently prevents calling free() directly. > From 0d18b70a8821ab2fc58b5ed592ed611e05a29c7f Mon Sep 17 00:00:00 2001 > From: Jim Meyering > Date: Mon, 3 Jan 2011 16:52:37 +0100 > Subject: [PATCH 1/2] discourage unnecessary use of if before free > > * README.Portability: Explain why "if (P) free (P)" is best avoided. > --- > =C2=A0gcc/README.Portability | =C2=A0 23 ++++++++++++++++------- > =C2=A01 files changed, 16 insertions(+), 7 deletions(-) > > diff --git a/gcc/README.Portability b/gcc/README.Portability > index 32a33e2..e099a3f 100644 > --- a/gcc/README.Portability > +++ b/gcc/README.Portability > @@ -51,14 +51,24 @@ foo (bar, ) > =C2=A0needs to be coded in some other way. > > > -free and realloc > ----------------- > +Avoid unnecessary test before free > +---------------------------------- > > -Some implementations crash upon attempts to free or realloc the null > -pointer. =C2=A0Thus if mem might be null, you need to write > +Since SunOS 4 stopped being a reasonable portability target, > +(which happened around 2007) there has been no need to guard > +against "free (NULL)". =C2=A0Thus, any guard like the following > +constitutes a redundant test: > > - =C2=A0if (mem) > - =C2=A0 =C2=A0free (mem); > + =C2=A0if (P) > + =C2=A0 =C2=A0free (P); > + > +It is better to avoid the test.[*] > +Instead, simply free P, regardless of whether it is NULL. > + > +[*] However, if your profiling exposes a test like this in a > +performance-critical loop, say where P is nearly always NULL, and > +the cost of calling free on a NULL pointer would be prohibitively > +high, please let us know. Instead of "please let us know", maybe recommend using __builtin_expect instead? E.g. something like if (__builtin_expect (ptr !=3D NULL, 0)) free (ptr); --=20 Janne Blomqvist