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