tech-userlevel archive

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

Incompat change (6 weeks ago) in stdio breaks formail (from procmail)



A seemingly inoccuous change in the latest version of lib;libc/stdio/stdio.c
made in late March ...

--- stdio.c     2012/03/20 01:42:59     1.20
+++ stdio.c     2012/03/27 15:05:42     1.21

-               (void) lseek(__sfileno(fp), (off_t)0, SEEK_END);
+               if (lseek(__sfileno(fp), (off_t)0, SEEK_END) == (off_t)-1)
+                       return -1;

which looks like it should obviously be the correct thing to do
(as after all, we should always check syscall errors...) but turns
out to cause problems.

This seek is done only if the (internal) __SAPP flag is set, which is
only ever set in the case of fdopen(fd, "a").   The problem for formail
is that it does this precise operation on a fd that (in the way I use
it anyway) had fd being the (write half of) the result from pipe(2).

I am not sure why formail is using "a" mode, rather than "w" but it might
be that the same fdopen() is also used where the fd came from different
origins, and append mode is actually wanted (formail is absurdly difficult
to decipher...).   In any case, this code has worked (and still works on all
NetBSD except current) for ages, and apparently works on all other systems
(the procmail stuff seems to have hacks to deal with just about every quirk in
every system ever seen, but it has nothing for this one).

Two solutions (other than a possible patch to formail - which would take some
investigation to validate the correctness of) seem possible to me.

One would be to simply revert that change, and ignore errors from the lseek()
like was always done before.

Another possibility would be to check for ESPIPE errno, and ignore that
particular error (anything that gives ESPIPE is unseekable, hence by
definition, all writes must be at the end - nothing else makes sense - and
so the lseek() is just a waste of time in that case).   If that's the
solution, I'd check for ESPIPE, if the lseek() fails, and if that's
the error, clear __SAPP (no point ever trying the lseek() again on
that fd) and continue - return the error for other errors (though I am
not sure what other errors lseek() could ever happen - the man page
mentions just ESPIPE, EBADF (which would be detected by the immediately
following write() sys call, so ignoring that one harms nothing) and
EINVAL, which cannot happen for this particular call.

That suggests that perhaps just reverting the change, and simply ignoring
the error is a better change.

Opinions?

kre


Home | Main Index | Thread Index | Old Index