Re: [PATCH 10/26] Check for EOF while parsing mails
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Apr 28, 2017, 10:44 UTC
- Message-ID
- <alpine.DEB.2.20.1704281241260.3480@virtualbox>
- In-Reply-To
- <20170427062114.p367j6rojnw4aj5r@sigill.intra.peff.net>
Hi Peff,
On Thu, 27 Apr 2017, Jeff King wrote:
Show 15 quoted lines
> On Wed, Apr 26, 2017 at 10:20:16PM +0200, Johannes Schindelin wrote:
>
> > diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c
> > index 30681681c13..c0d88f97512 100644
> > --- a/builtin/mailsplit.c
> > +++ b/builtin/mailsplit.c
> > @@ -232,7 +232,7 @@ static int split_mbox(const char *file, const char *dir, int allow_bare,
> >
> > do {
> > peek = fgetc(f);
> > - } while (isspace(peek));
> > + } while (peek >= 0 && isspace(peek));
> > ungetc(peek, f);
>
> Are we guaranteed that EOF is a negative number?No, you're right.
> Also, what is the behavior of ungetc when we pass it EOF?
According to the documentation, it would cast EOF to an unsigned char and push that back. Definitely incorrect.
> It looks like POSIX does what we want (pushing EOF is a noop, and the > stream retains its feof() status), but I don't know if there are other > implementations to worry about.
That's not what my man page here says:
ungetc() pushes c back to stream, cast to unsigned char, where it is available for subsequent read operations. Pushed-back characters will be returned in reverse order; only one pushback is guaranteed.
Show 11 quoted lines
> Perhaps:
>
> /* soak up whitespace */
> while ((peek = fgetc(f)) != EOF) {
> if (!isspace(peek)) {
> ungetc(peek, f);
> break;
> }
> }
>
> would be more portable.True. I changed it slightly differently, please see my reply to Hannes.
Thanks, Dscho