Re: [PATCH 09/23] expire_reflog(): extract two policy-related functions

2014-12-05 Thread Stefan Beller
On Fri, Dec 05, 2014 at 12:08:21AM +0100, Michael Haggerty wrote:
 Extract two functions, reflog_expiry_prepare() and
 reflog_expiry_cleanup(), from expire_reflog(). This is a further step
 towards separating the code for deciding on expiration policy from the
 code that manages the physical expiration.
 
 This change requires a couple of local variables from expire_reflog()
 to be turned into fields of struct expire_reflog_cb. More
 reorganization of the callback data will follow in later commits.
 
 Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
Reviewed-by: Stefan Beller sbel...@google.com

 ---
 In fact, the work done in reflog_expire_cleanup() doesn't really need
 to be done via a callback, because it doesn't need to be done while
 the reference lock is held. But the symmetry between prepare and
 cleanup is kindof nice. Perhaps some future policy decision will want
 to do some final work under the reference lock?
 
 But it would be easy to get rid of this third callback function and
 have the callers do the work themselves after calling expire_reflog().
 I don't have a string feeling either way.
 
  builtin/reflog.c | 94 
 +++-
  1 file changed, 52 insertions(+), 42 deletions(-)
 
 diff --git a/builtin/reflog.c b/builtin/reflog.c
 index 7bc6e0f..ebfa635 100644
 --- a/builtin/reflog.c
 +++ b/builtin/reflog.c
 @@ -43,6 +43,8 @@ struct expire_reflog_cb {
   unsigned long mark_limit;
   struct cmd_reflog_expire_cb *cmd;
   unsigned char last_kept_sha1[20];
 + struct commit *tip_commit;
 + struct commit_list *tips;
  };
  
  struct collected_reflog {
 @@ -363,6 +365,54 @@ static int push_tip_to_list(const char *refname, const 
 unsigned char *sha1, int
   return 0;
  }
  
 +static void reflog_expiry_prepare(const char *refname,
 +   const unsigned char *sha1,
 +   struct expire_reflog_cb *cb)
 +{
 + if (!cb-cmd-expire_unreachable || !strcmp(refname, HEAD)) {
 + cb-tip_commit = NULL;
 + cb-unreachable_expire_kind = UE_HEAD;
 + } else {
 + cb-tip_commit = lookup_commit_reference_gently(sha1, 1);
 + if (!cb-tip_commit)
 + cb-unreachable_expire_kind = UE_ALWAYS;
 + else
 + cb-unreachable_expire_kind = UE_NORMAL;
 + }
 +
 + if (cb-cmd-expire_unreachable = cb-cmd-expire_total)
 + cb-unreachable_expire_kind = UE_ALWAYS;
 +
 + cb-mark_list = NULL;
 + cb-tips = NULL;
 + if (cb-unreachable_expire_kind != UE_ALWAYS) {
 + if (cb-unreachable_expire_kind == UE_HEAD) {
 + struct commit_list *elem;
 + for_each_ref(push_tip_to_list, cb-tips);
 + for (elem = cb-tips; elem; elem = elem-next)
 + commit_list_insert(elem-item, cb-mark_list);
 + } else {
 + commit_list_insert(cb-tip_commit, cb-mark_list);
 + }
 + cb-mark_limit = cb-cmd-expire_total;
 + mark_reachable(cb);
 + }
 +}
 +
 +static void reflog_expiry_cleanup(struct expire_reflog_cb *cb)
 +{
 + if (cb-unreachable_expire_kind != UE_ALWAYS) {
 + if (cb-unreachable_expire_kind == UE_HEAD) {
 + struct commit_list *elem;
 + for (elem = cb-tips; elem; elem = elem-next)
 + clear_commit_marks(elem-item, REACHABLE);
 + free_commit_list(cb-tips);
 + } else {
 + clear_commit_marks(cb-tip_commit, REACHABLE);
 + }
 + }
 +}
 +
  static struct lock_file reflog_lock;
  
  static int expire_reflog(const char *refname, const unsigned char *sha1, 
 void *cb_data)
 @@ -371,8 +421,6 @@ static int expire_reflog(const char *refname, const 
 unsigned char *sha1, void *c
   struct expire_reflog_cb cb;
   struct ref_lock *lock;
   char *log_file;
 - struct commit *tip_commit;
 - struct commit_list *tips;
   int status = 0;
  
   memset(cb, 0, sizeof(cb));
 @@ -400,47 +448,9 @@ static int expire_reflog(const char *refname, const 
 unsigned char *sha1, void *c
  
   cb.cmd = cmd;
  
 - if (!cmd-expire_unreachable || !strcmp(refname, HEAD)) {
 - tip_commit = NULL;
 - cb.unreachable_expire_kind = UE_HEAD;
 - } else {
 - tip_commit = lookup_commit_reference_gently(sha1, 1);
 - if (!tip_commit)
 - cb.unreachable_expire_kind = UE_ALWAYS;
 - else
 - cb.unreachable_expire_kind = UE_NORMAL;
 - }
 -
 - if (cmd-expire_unreachable = cmd-expire_total)
 - cb.unreachable_expire_kind = UE_ALWAYS;
 -
 - cb.mark_list = NULL;
 - tips = NULL;
 - if (cb.unreachable_expire_kind != UE_ALWAYS) {
 - if (cb.unreachable_expire_kind == UE_HEAD) {
 -   

[PATCH 09/23] expire_reflog(): extract two policy-related functions

2014-12-04 Thread Michael Haggerty
Extract two functions, reflog_expiry_prepare() and
reflog_expiry_cleanup(), from expire_reflog(). This is a further step
towards separating the code for deciding on expiration policy from the
code that manages the physical expiration.

This change requires a couple of local variables from expire_reflog()
to be turned into fields of struct expire_reflog_cb. More
reorganization of the callback data will follow in later commits.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
In fact, the work done in reflog_expire_cleanup() doesn't really need
to be done via a callback, because it doesn't need to be done while
the reference lock is held. But the symmetry between prepare and
cleanup is kindof nice. Perhaps some future policy decision will want
to do some final work under the reference lock?

But it would be easy to get rid of this third callback function and
have the callers do the work themselves after calling expire_reflog().
I don't have a string feeling either way.

 builtin/reflog.c | 94 +++-
 1 file changed, 52 insertions(+), 42 deletions(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index 7bc6e0f..ebfa635 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -43,6 +43,8 @@ struct expire_reflog_cb {
unsigned long mark_limit;
struct cmd_reflog_expire_cb *cmd;
unsigned char last_kept_sha1[20];
+   struct commit *tip_commit;
+   struct commit_list *tips;
 };
 
 struct collected_reflog {
@@ -363,6 +365,54 @@ static int push_tip_to_list(const char *refname, const 
unsigned char *sha1, int
return 0;
 }
 
+static void reflog_expiry_prepare(const char *refname,
+ const unsigned char *sha1,
+ struct expire_reflog_cb *cb)
+{
+   if (!cb-cmd-expire_unreachable || !strcmp(refname, HEAD)) {
+   cb-tip_commit = NULL;
+   cb-unreachable_expire_kind = UE_HEAD;
+   } else {
+   cb-tip_commit = lookup_commit_reference_gently(sha1, 1);
+   if (!cb-tip_commit)
+   cb-unreachable_expire_kind = UE_ALWAYS;
+   else
+   cb-unreachable_expire_kind = UE_NORMAL;
+   }
+
+   if (cb-cmd-expire_unreachable = cb-cmd-expire_total)
+   cb-unreachable_expire_kind = UE_ALWAYS;
+
+   cb-mark_list = NULL;
+   cb-tips = NULL;
+   if (cb-unreachable_expire_kind != UE_ALWAYS) {
+   if (cb-unreachable_expire_kind == UE_HEAD) {
+   struct commit_list *elem;
+   for_each_ref(push_tip_to_list, cb-tips);
+   for (elem = cb-tips; elem; elem = elem-next)
+   commit_list_insert(elem-item, cb-mark_list);
+   } else {
+   commit_list_insert(cb-tip_commit, cb-mark_list);
+   }
+   cb-mark_limit = cb-cmd-expire_total;
+   mark_reachable(cb);
+   }
+}
+
+static void reflog_expiry_cleanup(struct expire_reflog_cb *cb)
+{
+   if (cb-unreachable_expire_kind != UE_ALWAYS) {
+   if (cb-unreachable_expire_kind == UE_HEAD) {
+   struct commit_list *elem;
+   for (elem = cb-tips; elem; elem = elem-next)
+   clear_commit_marks(elem-item, REACHABLE);
+   free_commit_list(cb-tips);
+   } else {
+   clear_commit_marks(cb-tip_commit, REACHABLE);
+   }
+   }
+}
+
 static struct lock_file reflog_lock;
 
 static int expire_reflog(const char *refname, const unsigned char *sha1, void 
*cb_data)
@@ -371,8 +421,6 @@ static int expire_reflog(const char *refname, const 
unsigned char *sha1, void *c
struct expire_reflog_cb cb;
struct ref_lock *lock;
char *log_file;
-   struct commit *tip_commit;
-   struct commit_list *tips;
int status = 0;
 
memset(cb, 0, sizeof(cb));
@@ -400,47 +448,9 @@ static int expire_reflog(const char *refname, const 
unsigned char *sha1, void *c
 
cb.cmd = cmd;
 
-   if (!cmd-expire_unreachable || !strcmp(refname, HEAD)) {
-   tip_commit = NULL;
-   cb.unreachable_expire_kind = UE_HEAD;
-   } else {
-   tip_commit = lookup_commit_reference_gently(sha1, 1);
-   if (!tip_commit)
-   cb.unreachable_expire_kind = UE_ALWAYS;
-   else
-   cb.unreachable_expire_kind = UE_NORMAL;
-   }
-
-   if (cmd-expire_unreachable = cmd-expire_total)
-   cb.unreachable_expire_kind = UE_ALWAYS;
-
-   cb.mark_list = NULL;
-   tips = NULL;
-   if (cb.unreachable_expire_kind != UE_ALWAYS) {
-   if (cb.unreachable_expire_kind == UE_HEAD) {
-   struct commit_list *elem;
-   for_each_ref(push_tip_to_list,