[PATCH 08/16] libiberty.h: Provide a CLEAR_VAR macro

David Malcolm dmalcolm@redhat.com
Tue Jun 2 01:23:00 GMT 2015


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



More information about the Binutils mailing list