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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: "Stefan Beller" <sbeller@xxxxxxxxxx>
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>

Has this been tested on Windows? I had a similar problem very recently
https://groups.google.com/forum/#!msg/git-for-windows/6LPxf9xZKhI/-s7XD18yCwAJ
where a bad rev-list-arg would cause the `bundle create` to die, and, on windows, leave the incomplete bundle file locked.

dscho suggested one possible solution for that, but I haven't had any time to try any patches.

---
bundle.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/bundle.c b/bundle.c
index 506ac49..fbc8593 100644
--- a/bundle.c
+++ b/bundle.c
@@ -407,6 +407,7 @@ int create_bundle(struct bundle_header *header, const char *path,
 int bundle_to_stdout;
 int ref_count = 0;
 struct rev_info revs;
+ int ret = -1;

 bundle_to_stdout = !strcmp(path, "-");
 if (bundle_to_stdout)
@@ -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);
+ 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;
}

int unbundle(struct bundle_header *header, int bundle_fd, int flags)
--
2.8.0.2.gb331331

--
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


--
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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]