On Thu, Feb 05, 2015 at 01:16:36PM -0800, Junio C Hamano wrote:
> Jeff King <peff@xxxxxxxx> 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...
Yeah, that was my thinking, and why I have doubts. Maybe a comment would
make more sense, like the patch below. I am also OK with just leaving
it as-is.
-- >8 --
Subject: [PATCH] config_buf_ungetc: document quirks in a comment
Our config_buf_ungetc implements just enough for the config
code to work. That's OK, but we would not want anyone to
mistakenly move it elsewhere as a general purpose ungetc.
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
config.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/config.c b/config.c
index 2c63099..089a94f 100644
--- a/config.c
+++ b/config.c
@@ -71,6 +71,12 @@ static int config_buf_fgetc(struct config_source *conf)
return EOF;
}
+/*
+ * Note that this is not a real ungetc replacement. It only rewinds
+ * the position, and ignores the "c" parameter, rather than
+ * putting it into our (const) buffer. That's good enough for
+ * the callers here, though.
+ */
static int config_buf_ungetc(int c, struct config_source *conf)
{
if (conf->u.buf.pos > 0)
--
2.3.0.rc1.287.g761fd19
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html