git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCHv2 1/4] notes: don't leak memory in git_config_get_notes_strategy

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Mar 30, 2016, 17:32 UTC
Message-ID
<CAPig+cTeVgyNHa5uHUMDZ6ifxyjk2wvm=jGd9tP1+BuBKKVR+g@mail.gmail.com>
In-Reply-To
<1459357518-14913-2-git-send-email-sbeller@google.com>
On Wed, Mar 30, 2016 at 1:05 PM, Stefan Beller <sbeller@google.com> wrote:
Show 25 quoted lines
> `value` is just a temporary scratchpad, so we need to make sure it doesn't
> leak. It is xstrdup'd in `git_config_get_string` and
> `parse_notes_merge_strategy` just compares the string against predefined
> values, so no need to keep it around longer. Make `value` non-const to
> avoid the cast in the free.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> diff --git a/builtin/notes.c b/builtin/notes.c
> @@ -744,13 +744,14 @@ static int merge_commit(struct notes_merge_options *o)
>  static int git_config_get_notes_strategy(const char *key,
>                                          enum notes_merge_strategy *strategy)
>  {
> -       const char *value;
> +       char *value;
>
> -       if (git_config_get_string_const(key, &value))
> +       if (git_config_get_string(key, &value))
>                 return 1;
>         if (parse_notes_merge_strategy(value, strategy))
>                 git_die_config(key, "unknown notes merge strategy %s", value);
>
> +       free(value);
>         return 0;
>  }

Hmm, I thought Peff's suggestion of using git_config_get_value() was accepted as superior since it avoids the allocation altogether, thus no need for free() and no leak.

Previous: Stefan BellerNext: Junio C Hamano
Message 3 of 20 in “[PATCHv2 0/4] Some cleanups”
  1. Stefan BellerMar 30, 2016
  2. 1/4 notes: don't leak memory in git_config_get_notes_strategyStefan Beller, Mar 30, 2016
  3. Eric SunshineMar 30, 2016
  4. Junio C HamanoMar 30, 2016
  5. Stefan BellerMar 30, 2016
  6. Jeff KingMar 31, 2016
  7. Junio C HamanoMar 31, 2016
  8. 2/4 abbrev_sha1_in_line: don't leak memoryStefan Beller, Mar 30, 2016
  9. Eric SunshineMar 30, 2016
  10. 3/4 bundle: don't leak an fd in case of early returnStefan Beller, Mar 30, 2016
  11. Jeff KingMar 30, 2016
  12. Eric SunshineMar 30, 2016
  13. Stefan BellerMar 31, 2016
  14. Philip OakleyMar 31, 2016
  15. Stefan BellerApr 1, 2016
  16. Philip OakleyApr 1, 2016
  17. 4/4 credential-cache, send_request: close fd when doneStefan Beller, Mar 30, 2016
  18. Jeff KingMar 30, 2016
  19. Stefan BellerMar 30, 2016
  20. Jeff KingMar 30, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.