Re: [PATCHv4 3/4] bundle: don't leak an fd in case of early return
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 1, 2016, 17:05 UTC
- Message-ID
- <xmqqzitdi7iq.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <1459470946-16522-4-git-send-email-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
Show 9 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> > ---
Thanks. I think this turned out to be the trickiest one to get right among the four and my reading of this round tells me that it addresses all the trickiness pointed out in the reviews.
Will replace.
Show 51 quoted lines
> bundle.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/bundle.c b/bundle.c
> index 506ac49..08e32ca 100644
> --- a/bundle.c
> +++ b/bundle.c
> @@ -435,12 +435,14 @@ 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) {
> + error(_("unrecognized argument: %s"), argv[1]);
> + goto err;
> + }
>
> object_array_remove_duplicates(&revs.pending);
>
> @@ -448,17 +450,23 @@ int create_bundle(struct bundle_header *header, const char *path,
> if (!ref_count)
> die(_("Refusing to create empty bundle."));
> else if (ref_count < 0)
> - return -1;
> + 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 -1;
> }
>
> int unbundle(struct bundle_header *header, int bundle_fd, int flags)