On Wed, Mar 30, 2016 at 1:05 PM, Stefan Beller <sbeller@xxxxxxxxxx> wrote:
> 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@xxxxxxxx>
> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
> ---
> 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?
> + 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;
> }
--
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