Ugh, I should learn not to send out patches way past my bed-time. This
should have been marked as RFC, and the subject should probably say
"1023 bytes", since one byte was needed for zero-termination.
Sorry for the noise.
On Wed, Apr 6, 2011 at 1:30 AM, Erik Faye-Lund <kusmabite@xxxxxxxxx> wrote:
> parse_value in config.c has a static buffer of 1024 bytes that it
> parse the value into. This can sometimes be a problem when a
> config file contains very long values.
>
> It's particularly amusing that git-config already is able to write
> such files, so it should probably be able to read them as well.
>
> Fix this by using a strbuf instead of a static buffer.
>
> Signed-off-by: Erik Faye-Lund <kusmabite@xxxxxxxxx>
> ---
>
> Here's a proper-ish submission with a commit message and all.
>
> The rather awkward return statement with strdup("") is because
> strbuf_detach returns NULL when there's nothing allocated. Even
> worse, it returns an uninitialized string if the string has been
> initialized with a non-zero 'hint'.
>
> Perhaps I should change it to return a heap-allocated, empty
> string in those cases instead of working around it here?
>
> config.c | 26 ++++++++++++++------------
> t/t1303-wacky-config.sh | 2 +-
> 2 files changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/config.c b/config.c
> index 0abcada..f882f7c 100644
> --- a/config.c
> +++ b/config.c
> @@ -133,23 +133,22 @@ static int get_next_char(void)
>
> static char *parse_value(void)
> {
> - static char value[1024];
> - int quote = 0, comment = 0, len = 0, space = 0;
> + struct strbuf value = STRBUF_INIT;
> + int quote = 0, comment = 0, space = 0;
>
> for (;;) {
> int c = get_next_char();
> - if (len >= sizeof(value) - 1)
> - return NULL;
> if (c == '\n') {
> if (quote)
> return NULL;
> - value[len] = 0;
> - return value;
> + return value.len ?
> + strbuf_detach(&value, NULL) :
> + strdup("");
Should have used xstrdup()...
> }
> if (comment)
> continue;
> if (isspace(c) && !quote) {
> - if (len)
> + if (value.len)
> space++;
> continue;
> }
> @@ -160,7 +159,7 @@ static char *parse_value(void)
> }
> }
> for (; space; space--)
> - value[len++] = ' ';
> + strbuf_addch(&value, ' ');
> if (c == '\\') {
> c = get_next_char();
> switch (c) {
> @@ -180,16 +179,17 @@ static char *parse_value(void)
> break;
> /* Reject unknown escape sequences */
> default:
> + strbuf_release(&value);
> return NULL;
> }
> - value[len++] = c;
> + strbuf_addch(&value, c);
> continue;
> }
> if (c == '"') {
> quote = 1-quote;
> continue;
> }
> - value[len++] = c;
> + strbuf_addch(&value, c);
> }
> }
>
> @@ -200,7 +200,7 @@ static inline int iskeychar(int c)
>
> static int get_value(config_fn_t fn, void *data, char *name, unsigned int len)
> {
> - int c;
> + int c, ret;
> char *value;
>
> /* Get the full name */
> @@ -226,7 +226,9 @@ static int get_value(config_fn_t fn, void *data, char *name, unsigned int len)
> if (!value)
> return -1;
> }
> - return fn(name, value, data);
> + ret = fn(name, value, data);
> + free(value);
> + return ret;
> }
>
> static int get_extended_base_var(char *name, int baselen, int c)
> diff --git a/t/t1303-wacky-config.sh b/t/t1303-wacky-config.sh
> index 080117c..46103a1 100755
> --- a/t/t1303-wacky-config.sh
> +++ b/t/t1303-wacky-config.sh
> @@ -44,7 +44,7 @@ LONG_VALUE=$(printf "x%01021dx a" 7)
> test_expect_success 'do not crash on special long config line' '
> setup &&
> git config section.key "$LONG_VALUE" &&
> - check section.key "fatal: bad config file line 2 in .git/config"
> + check section.key "$LONG_VALUE"
> '
>
> test_done
> --
> 1.7.4.msysgit.0.168.g33778
>
>
--
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