bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [bug-gnulib] clean-temp usage question


From: Bruno Haible
Subject: Re: [bug-gnulib] clean-temp usage question
Date: Sat, 7 Oct 2006 17:06:50 +0200
User-agent: KMail/1.9.1

Eric Blake wrote:
> Grammar: s/one can/can one/

Thanks, applied.

> > --- gnulib-20060928-modified/lib/clean-temp.c       2006-10-06 
> > 03:37:05.000000000 +0200
> > ***************
> > *** 41,46 ****
> > --- 41,50 ----
> >
> >   #define _(str) gettext (str)
> >
> > + #ifndef uintptr_t
> > + # define uintptr_t unsigned long
> > + #endif
> > +
> 
> Can't we just get this by including one of the gnulib headers that
> guarantees the correct types?  I'm a bit worried that there might be a
> platform with uintptr_t typedef'd to a different size integer, at which
> point this #define is broken.

This would be harmless. The uintptr_t here is only used to cast between
'int' and 'void *'. If it is the wrong size, the worst thing that can
happen is that gcc emits some warnings.

> > +   saved_errno = errno;
> > +   if (fp != NULL)
> > +     {
> > +       /* It is sufficient to register fileno (fp) instead of the entire 
> > fp,
> > +    because at cleanup time there is no need to do an fflush (fp); a
> > +    close (fileno (fp)) will be enough.  */
> 
> At fatal signal time, yes, calling fflush is unnecessary - the program is
> going to die from the signal anyways, so losing unflushed data does not
> hurt my feelings.  But it does mean that I must manually fflush/fclose all
> returned FILE*s before calling cleanup_temp_subdir (although it is good
> practice to do that anyways).

Yes, you will do it anyway. If you make up a temp file and you would
consider losing unflushed data a bug, there must be some program going
to read the data after this point. Which is impossible if you already
call cleanup_temp_subdir at this point.

> > +       /* No race condition here: we assume a single-threaded program, 
> > hence
> > +    fd cannot be re-opened here.  */
> 
> State that assumption in the comments for create_temp_dir.

gnulib modules are not MT-safe by default. We can at some point start
adding comments "this API is MT-safe" in each module, when needed.

> Is it worth documenting that given a FILE* from fopen_temp, I can do:
> fd=fileno(fp); close_stream(fp); close_temp(fd);
> rather than:
> fclose_temp(fp);

In which situations would it be useful to know this? Few programs hack
around with fileno and freopen.

> Once fp is closed, I can't call fclose_temp because the storage underlying
> fp is no longer valid, but if you provide a guarantee that I only have to
> unregister the fd, then I can choose between close_stream (which M4 is
> already using) or converting over to fwriteerror for disposing of
> temporary FILE* objects.

Use close_stream_temp. Simple enough.

Bruno




reply via email to

[Prev in Thread] Current Thread [Next in Thread]