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

Re: [PATCHv2 3/4] bundle: don't leak an fd in case of early return

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Mar 30, 2016, 17:41 UTC
Message-ID
<CAPig+cSE5wVNNwiwkYoOXnfDKUO3tBKqUUbqBHPCTA5ibj-kNg@mail.gmail.com>
In-Reply-To
<1459357518-14913-4-git-send-email-sbeller@google.com>
On Wed, Mar 30, 2016 at 1:05 PM, Stefan Beller <sbeller@google.com> wrote:
Show 36 quoted lines
> In successful operation `write_pack_data` will close the `bundle_fd`,
> but when we exit early, we need to take care of the file descriptor
> as well as the lock file ourselves. The lock file may be deleted at the
> end of running the program, but we are in library code, so we should
> not rely on that.
>
> Helped-by: Jeff King <peff@peff.net>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> diff --git a/bundle.c b/bundle.c
> @@ -435,30 +436,40 @@ int create_bundle(struct bundle_header *header, const char *path,
>
>         /* write prerequisites */
>         if (compute_and_write_prerequisites(bundle_fd, &revs, argc, argv))
> -               return -1;
> +               goto err;
>
>         argc = setup_revisions(argc, argv, &revs, NULL);
>
> -       if (argc > 1)
> -               return error(_("unrecognized argument: %s"), argv[1]);
> +       if (argc > 1) {
> +               ret = error(_("unrecognized argument: %s"), argv[1]);
> +               goto err;
> +       }
>
>         object_array_remove_duplicates(&revs.pending);
>
>         ref_count = write_bundle_refs(bundle_fd, &revs);
>         if (!ref_count)
>                 die(_("Refusing to create empty bundle."));
> -       else if (ref_count < 0)
> -               return -1;
> +       else if (ref_count < 0) {
> +               if (!bundle_to_stdout)
> +                       close(bundle_fd);
Why is this close() here considering that it gets closed by the 'err' path?
Show 19 quoted lines
> +               goto err;
> +       }
>
>         /* write pack */
>         if (write_pack_data(bundle_fd, &revs))
> -               return -1;
> +               goto err;
>
>         if (!bundle_to_stdout) {
>                 if (commit_lock_file(&lock))
>                         die_errno(_("cannot create '%s'"), path);
>         }
>         return 0;
> +err:
> +       if (!bundle_to_stdout)
> +               close(bundle_fd);
> +       rollback_lock_file(&lock);
> +       return ret;
>  }
Previous: Jeff KingNext: Stefan Beller
Message 12 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.