* [PATCH] files_for_each_reflog_ent_reverse(): close stream and free strbuf on error
@ 2017-04-16 16:55 René Scharfe
2017-04-17 7:05 ` Michael Haggerty
0 siblings, 1 reply; 2+ messages in thread
From: René Scharfe @ 2017-04-16 16:55 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Michael Haggerty
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 <l.s.r@web.de>
---
refs/files-backend.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 50188e92f9..2889f21568 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3159,8 +3159,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;
@@ -3170,13 +3170,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;
--
2.12.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] files_for_each_reflog_ent_reverse(): close stream and free strbuf on error
2017-04-16 16:55 [PATCH] files_for_each_reflog_ent_reverse(): close stream and free strbuf on error René Scharfe
@ 2017-04-17 7:05 ` Michael Haggerty
0 siblings, 0 replies; 2+ messages in thread
From: Michael Haggerty @ 2017-04-17 7:05 UTC (permalink / raw)
To: René Scharfe, Git List; +Cc: Junio C Hamano
On 04/16/2017 06:55 PM, René Scharfe wrote:
> Exit the loop orderly through the cleanup code, instead of dashing out
> with logfp still open and sb leaking.
>
> Found with Cppcheck.
Nice catch.
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
> refs/files-backend.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 50188e92f9..2889f21568 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -3159,8 +3159,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);
It seems odd to call `ftell()` in the case that `fseek()` has failed,
but it should be harmless.
> while (!ret && 0 < pos) {
> int cnt;
> @@ -3170,13 +3170,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;
>
Reviewed-by: Michael Haggerty <mhagger@alum.mit.edu>
Michael
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-04-17 7:05 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-16 16:55 [PATCH] files_for_each_reflog_ent_reverse(): close stream and free strbuf on error René Scharfe
2017-04-17 7:05 ` Michael Haggerty
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).