Jeff King <p...@peff.net> writes:
> 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.
I am not sure, either. If this were to become stdio emulator over
random in-core data used throughout the system, perhaps.
But in its current form it is tied to the implementation of config.c
very strongly, so...
> -- >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 <p...@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;
> }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
