Hi Peff,

On Thu, 27 Apr 2017, Jeff King wrote:

> 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.

> 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

Reply via email to