Re: [PATCH] config: do not ungetc EOF

> On Thu, Feb 05, 2015 at 01:53:27AM -0500, Jeff King wrote:
>
>> I also notice that config_buf_ungetc does not actually ungetc the
>> character we give it; it just rewinds one character in the stream. This
>> is fine, because we always feed the last-retrieved character. I dunno if
>> it is worth fixing (it also would have fixed this infinite loop, but for
>> the wrong reason; we would have stuck "-1" back into the stream, and
>> retrieved it on the next fgetc rather than the same '\r' over and over).
>
> Here's a patch to deal with that. I'm not sure if it's worth doing or
> not.

> -- >8 --
> Subject: [PATCH] config_buf_ungetc: warn when pushing back a random character
>
> Our config code simulates a stdio stream around a buffer,
> but our fake ungetc() does not behave quite like the real
> one. In particular, we only rewind the position by one
> character, but do _not_ actually put the character from the
> caller into position.
>
> It turns out that this does not matter, because we only ever
> push back the character we just read. In other words, such
> an assignment would be a noop. But because the function is
> called ungetc, and because it takes a character parameter,
> it is a mistake waiting to happen.
>
> Actually assigning the character into the buffer would be
> ideal, but our pointer is actually a "const" copy of the
> buffer. We do not know who the real owner of the buffer is
> in this code, and would not want to munge their contents.
>
> Instead, we can simply add an assertion that matches what
> the current caller does, and will let us know if new callers
> are added that violate the contract.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  config.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/config.c b/config.c
> index 2c63099..b74cc47 100644
> --- a/config.c
> +++ b/config.c
> @@ -73,8 +73,12 @@ static int config_buf_fgetc(struct config_source *conf)
>  
>  static int config_buf_ungetc(int c, struct config_source *conf)
>  {
> -	if (conf->u.buf.pos > 0)
> -		return conf->u.buf.buf[--conf->u.buf.pos];
> +	if (conf->u.buf.pos > 0) {
> +		conf->u.buf.pos--;
> +		if (conf->u.buf.buf[conf->u.buf.pos] != c)
> +			die("BUG: config_buf can only ungetc the same character");
> +		return c;
> +	}
>  
>  	return EOF;
>  }