From ac8ce18d89acccaa7a66900adff75d4aeb6ec80b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sun, 16 Apr 2017 18:55:30 +0200 Subject: am: close stream on error, but not stdin Avoid closing stdin, but do close an actual input file on error exit. Found with Cppcheck. Signed-off-by: Rene Scharfe Reviewed-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/builtin/am.c b/builtin/am.c index 31fb605..1729600 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -762,14 +762,18 @@ static int split_mail_conv(mail_conv_fn fn, struct am_state *state, mail = mkpath("%s/%0*d", state->dir, state->prec, i + 1); out = fopen(mail, "w"); - if (!out) + if (!out) { + if (in != stdin) + fclose(in); return error_errno(_("could not open '%s' for writing"), mail); + } ret = fn(out, in, keep_cr); fclose(out); - fclose(in); + if (in != stdin) + fclose(in); if (ret) return error(_("could not parse patch '%s'"), *paths); -- cgit v0.10.2-6-g49f6 From be686f03e0f4c4f14f1d4ae9b1b35836168a0a4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sun, 16 Apr 2017 18:55:46 +0200 Subject: files_for_each_reflog_ent_reverse(): close stream and free strbuf on error Exit the loop orderly through the cleanup code, instead of dashing out with logfp still open and sb leaking. Found with Cppcheck. Signed-off-by: Rene Scharfe Reviewed-by: Jeff King Reviewed-by: Michael Haggerty Signed-off-by: Junio C Hamano diff --git a/refs/files-backend.c b/refs/files-backend.c index c041d4b..10ba254 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3169,8 +3169,8 @@ static int files_for_each_reflog_ent_reverse(struct ref_store *ref_store, /* Jump to the end */ if (fseek(logfp, 0, SEEK_END) < 0) - return error("cannot seek back reflog for %s: %s", - refname, strerror(errno)); + ret = error("cannot seek back reflog for %s: %s", + refname, strerror(errno)); pos = ftell(logfp); while (!ret && 0 < pos) { int cnt; @@ -3180,13 +3180,17 @@ static int files_for_each_reflog_ent_reverse(struct ref_store *ref_store, /* Fill next block from the end */ cnt = (sizeof(buf) < pos) ? sizeof(buf) : pos; - if (fseek(logfp, pos - cnt, SEEK_SET)) - return error("cannot seek back reflog for %s: %s", - refname, strerror(errno)); + if (fseek(logfp, pos - cnt, SEEK_SET)) { + ret = error("cannot seek back reflog for %s: %s", + refname, strerror(errno)); + break; + } nread = fread(buf, cnt, 1, logfp); - if (nread != 1) - return error("cannot read %d bytes from reflog for %s: %s", - cnt, refname, strerror(errno)); + if (nread != 1) { + ret = error("cannot read %d bytes from reflog for %s: %s", + cnt, refname, strerror(errno)); + break; + } pos -= cnt; scanp = endp = buf + cnt; -- cgit v0.10.2-6-g49f6 From fa1912c89a72fbd94591f4f5d522e5867ffe9bb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sun, 16 Apr 2017 18:55:58 +0200 Subject: server-info: avoid calling fclose(3) twice in update_info_file() If an error occurs when or after closing the stream we call fclose(3) again in the error handler. The second call can exhibit undefined behavior, so make sure to call fclose(3) at most once. Also avoid calling close(2) after fd has been successfully associated with the stream, as fclose(3) has become responsible for doing that beyond this point. Found with Cppcheck. Signed-off-by: Rene Scharfe Reviewed-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/server-info.c b/server-info.c index 7bc4e75..f6c1a3d 100644 --- a/server-info.c +++ b/server-info.c @@ -14,19 +14,21 @@ static int update_info_file(char *path, int (*generate)(FILE *)) char *tmp = mkpathdup("%s_XXXXXX", path); int ret = -1; int fd = -1; - FILE *fp = NULL; + FILE *fp = NULL, *to_close; safe_create_leading_directories(path); fd = git_mkstemp_mode(tmp, 0666); if (fd < 0) goto out; - fp = fdopen(fd, "w"); + to_close = fp = fdopen(fd, "w"); if (!fp) goto out; + fd = -1; ret = generate(fp); if (ret) goto out; - if (fclose(fp)) + fp = NULL; + if (fclose(to_close)) goto out; if (adjust_shared_perm(tmp) < 0) goto out; -- cgit v0.10.2-6-g49f6