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

include/ChangeLog:
	* libiberty.h (CLEAR_VAR): New macro.
---
 include/libiberty.h | 4 ++++
 1 file changed, 4 insertions(+)

> +/* 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.

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.

Also, pedantically speaking, "all bits zero" is not guaranteed to be
the same as float 0.0, double 0.0, or pointer (void *)0.

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

> 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.

Another option is to save the state as it was initialized, and restore
it as needed.  That way, structures that don't start off all zero will
get "reset" properly.  In this case, you're just talking about a
simple assignment:

  saved_vars = my_vars;
  . . .
  my_vars = saved_vars;

In addition to simplicity, the compiler will check the types for you
to make sure you don't goof up.

> Are you thinking of vtables here, "public" vs "private", or of
> inheritance?

Yes.

>  FWIW I'm only using this from within binutils, which AFAIK is pure
> C (I'm new to binutils).

Ah, but you're putting the macro in libiberty, which *is* used by C++
files.

> > 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;

That's not what you're doing, what you're doing is setting it to all
zero, even if the data was non-zero at the start.

> 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)

bfd.h might be an option, since it's common to most binutils programs.