[Gc] boehm-gc patch

Boehm, Hans hans.boehm at hp.com
Mon Dec 14 11:18:38 PST 2009


> From: Matthias Andree
> Sent: Monday, December 14, 2009 3:50 AM
> To: gc at napali.hpl.hp.com
> Subject: Re: [Gc] boehm-gc patch
> 
> Am 14.12.2009 12:45, schrieb Jack Andrews:
> > hi,
> > 
> >>> --- cord/cordxtra.c   10 Oct 2005 22:33:34 -0000      1.1.1.1
> >>> +++ cord/cordxtra.c   14 Dec 2009 00:31:33 -0000
> >>> @@ -574,7 +574,8 @@ CORD CORD_from_file_lazy_inner(FILE * f,
> >>>       /* world is multithreaded.                      */
> >>>       char buf[1];
> >>>
> >>> -     (void) fread(buf, 1, 1, f);
> >>> +     /* This `if' construct avoids a warning from GCC. */
> >>> +     if (fread(buf, 1, 1, f)) { }
> >>>       rewind(f);
> >>>      }
> >>>      state -> lf_file = f;
> >>
> >> This is rather horrid.  Do we really care so much about compiler 
> >> warnings that we will put such contorted code into our programs?
> > 
> > if fread() fails, shouldn't there be error handling?
> > 
> > if not, define a macro
> >   #define IGNORE(x) if(!(x)); /* ignore the result of x*/
> 
> You can just cast the return type to void, which - to me - 
> seems a bit more obvious. (No need to discuss this, just 
> wanted to propose this alternative; everybody can pick the 
> version (s)he likes best.) 
The current code already casts the result to void.  The fact that gcc still gives a
warning seems mildly wrong to me.  The value is clearly being intentionally ignored.

Since the point of the fread call here is to force buffer allocation, and it's very likely that any error will be reported later anyway, it seems to me that the current code is basically fine, and I'm not enthusiastic about obscuring it.  The IGNORE macro seems even worse to me.  Now a reader needs to look up the macro to understand the code.  If we really need to make the warning go away, I'd use

if (fread(buf, 1, 1, f) == 0) ABORT("CORD_from_file_lazy: fread failed")

which is consistent with the style in the rest of the code here, and seems only very marginally worse than the current code.  (And it's not clear we can do anything better than abort if this happens.)

Hans


More information about the Gc mailing list