Re: [PATCH 06/23] expire_reflog(): exit early if the reference has no reflog

2014-12-05 Thread Michael Haggerty
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

2014-12-04 Thread Michael Haggerty
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

2014-12-04 Thread Jonathan Nieder
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

2014-12-04 Thread Jonathan Nieder
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