Re: [PATCH 06/23] expire_reflog(): exit early if the reference has no reflog
On 12/05/2014 12:53 AM, Jonathan Nieder wrote: Michael Haggerty wrote: [Subject: expire_reflog(): exit early if the reference has no reflog] The caller moves on to expire other reflogs, so it's not exiting. return early, maybe? Except the function already returned early. I think the purpose of this patch is to simplify the no-reflog case by handling it separately. Anyway, that's just nitpicking about the subject line. With s/exit/return/ it should be clear that this is a refactoring change, which for someone looking at the shortlog is the important thing. Good suggestion. I will change the commit message. Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/23] expire_reflog(): exit early if the reference has no reflog
There is very little cleanup needed if the reference has no reflog. If we move the initialization of log_file down a bit, there's even less. So instead of jumping to the cleanup code at the end of the function, just do the cleanup and return inline. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- builtin/reflog.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/builtin/reflog.c b/builtin/reflog.c index df302f4..a282e60 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -368,9 +368,11 @@ static int expire_reflog(const char *refname, const unsigned char *sha1, void *c lock = lock_any_ref_for_update(refname, sha1, 0, NULL); if (!lock) return error(cannot lock ref '%s', refname); + if (!reflog_exists(refname)) { + unlock_ref(lock); + return 0; + } log_file = git_pathdup(logs/%s, refname); - if (!reflog_exists(refname)) - goto finish; if (!cmd-dry_run) { newlog_path = git_pathdup(logs/%s.lock, refname); cb.newlog = fopen(newlog_path, w); @@ -419,7 +421,7 @@ static int expire_reflog(const char *refname, const unsigned char *sha1, void *c clear_commit_marks(tip_commit, REACHABLE); } } - finish: + if (cb.newlog) { if (fclose(cb.newlog)) { status |= error(%s: %s, strerror(errno), -- 2.1.3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/23] expire_reflog(): exit early if the reference has no reflog
Michael Haggerty wrote: --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -368,9 +368,11 @@ static int expire_reflog(const char *refname, const unsigned char *sha1, void *c lock = lock_any_ref_for_update(refname, sha1, 0, NULL); if (!lock) return error(cannot lock ref '%s', refname); + if (!reflog_exists(refname)) { + unlock_ref(lock); + return 0; + } log_file = git_pathdup(logs/%s, refname); - if (!reflog_exists(refname)) - goto finish; Okay. Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/23] expire_reflog(): exit early if the reference has no reflog
Michael Haggerty wrote: [Subject: expire_reflog(): exit early if the reference has no reflog] The caller moves on to expire other reflogs, so it's not exiting. return early, maybe? Except the function already returned early. I think the purpose of this patch is to simplify the no-reflog case by handling it separately. Anyway, that's just nitpicking about the subject line. With s/exit/return/ it should be clear that this is a refactoring change, which for someone looking at the shortlog is the important thing. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html