From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 129128 invoked by alias); 2 Jun 2015 01:23:57 -0000 Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org Received: (qmail 129109 invoked by uid 89); 2 Jun 2015 01:23:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.1 required=5.0 tests=AWL,BAYES_50,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 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 (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 02 Jun 2015 01:23:56 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id E8218398A03; Tue, 2 Jun 2015 01:23:54 +0000 (UTC) Received: from [10.3.228.162] (vpn-228-162.phx2.redhat.com [10.3.228.162]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t521Nr2o030867; Mon, 1 Jun 2015 21:23:54 -0400 Message-ID: <1433207798.12727.49.camel@surprise> Subject: Re: [PATCH 08/16] libiberty.h: Provide a CLEAR_VAR macro From: David Malcolm To: DJ Delorie Cc: gcc-patches@gcc.gnu.org, binutils@sourceware.org Date: Tue, 02 Jun 2015 01:23:00 -0000 In-Reply-To: <201506012147.t51Ll08P015579@greed.delorie.com> References: <1433192664-50156-1-git-send-email-dmalcolm@redhat.com> <1433192664-50156-9-git-send-email-dmalcolm@redhat.com> <201506012147.t51Ll08P015579@greed.delorie.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-06/txt/msg00037.txt.bz2 On Mon, 2015-06-01 at 17:47 -0400, DJ Delorie wrote: > > +/* Fill an lvalue with zero bits. */ > > +#define CLEAR_VAR(S) \ > > + do { memset (&(S), 0, sizeof (S)); } while (0) > > Hmmm... I don't see the point in this. The macro is almost as long as > a bare memset() would be, and replaces a well-known function with > something unknown outside this project. It neither hides a bug in an > OS nor provides a common way to handle a difficult task across > platforms. Thanks for looking at this. FWIW I'm not in love with the name of the macro, but I find it useful. In the initial version of patches 10 and 12 (state purging within "gas" and "ld" subdirs) I didn't have this macro, and had about 30 memset invocations. There are a couple of ways to mess up such code; consider: memset (&statement_list, 0, sizeof (statement_list)); memset (&stat_save, 0, sizeof (statement_list)); where a bug is hiding: the 2nd memset is using the wrong sizeof, leading to only part of it being zeroed. Having a macro for this makes the code shorter: CLEAR_VAR (statement_list); CLEAR_VAR (stat_save); and thus more reliable (my eyes glaze over looking at all the memsets in the more involved examples). There's probably another way to mess this up, by forgetting the & on the first param. > You also do NOT want to use memset to zero out a C++ structure that > contains more than just "plain old data" - you could easily corrupt > the structure's hidden data. Are you thinking of vtables here, "public" vs "private", or of inheritance? FWIW I'm only using this from within binutils, which AFAIK is pure C (I'm new to binutils). > Also, pedantically speaking, "all bits zero" is not guaranteed to be > the same as float 0.0, double 0.0, or pointer (void *)0. The purpose of the code is to restore the state back to what it was when the library was first loaded; in the various foo_c_finalize () functions in patches 10 and 12 I try to only use CLEAR_VAR for structs, and instead spell out NULL etc for ptrs. Perhaps CLEAR_STRUCT () might be a better name? Also, if this isn't so much a libiberty thing (not being an OS-compatibility thing), is there a place to put it in internal support headers? (in the sense that this would be an implementation detail, used by both gas and ld) Thanks Dave