Re: [PATCH] refs.c: change read_ref_at to use the reflog iterators
Thanks.
On Mon, Jun 2, 2014 at 2:21 PM, Junio C Hamano gits...@pobox.com wrote:
Ronnie Sahlberg sahlb...@google.com writes:
read_ref_at has its own parsing of the reflog file for no really good reason
so lets change this to use the existing reflog iterators. This removes one
instance where we manually unmarshall the reflog file format.
Log messages for errors are changed slightly. We no longer print the file
name for the reflog, instead we refer to it as 'Log for ref refname'.
This might be a minor useability regression, but I don't really think so,
since
experienced users would know where the log is anyway and inexperienced users
would not know what to do about/how to repair 'Log ... has gap ...' anyway.
Adapt the t1400 test to handle the change in log messages.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c| 202
++
t/t1400-update-ref.sh | 4 +-
2 files changed, 107 insertions(+), 99 deletions(-)
diff --git a/refs.c b/refs.c
index 6898263..b45bb2f 100644
--- a/refs.c
+++ b/refs.c
@@ -2936,109 +2936,117 @@ static char *ref_msg(const char *line, const char
*endp)
return xmemdupz(line, ep - line);
}
If I am not mistaken, this function will become unused with this
rewrite. Let's drop it and justify it in the log message.
Done.
+struct read_ref_at_cb {
+ const char *refname;
+ unsigned long at_time;
+ int cnt;
+ int reccnt;
+ unsigned char *sha1;
+ int found_it;
+
+ char osha1[20];
+ char nsha1[20];
These should be unsigned char, shouldn't they?
Done.
+ for_each_reflog_ent_reverse(refname, read_ref_at_ent, cb);
+
+ if (!cb.reccnt)
+ die(Log for %s is empty., refname);
+ if (cb.found_it)
+ return 0;
+
+ for_each_reflog_ent(refname, read_ref_at_ent_oldest, cb);
Hmph.
We have already scanned the same reflog in the backward direction in
full. Do we need to make another call just to pick up the entry at
the beginning? Is this because the callback is not told that it is
fed the last entry (in other words, if there is some clue that this
is the last call from for-each-reflog-ent-reverse to the callback,
the callback could record the value it received in its cb-data)?
It is mainly because the callback does not provide the information
this is the last entry.
We could add a flag for that to the callback arguments but I don't
know if it is worth it for this special case
since read_ref_at() for a timestamp that is outside the reflog horizon
should be uncommon.
regards
ronnie sahlberg
--
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] refs.c: change read_ref_at to use the reflog iterators
read_ref_at has its own parsing of the reflog file for no really good reason
so lets change this to use the existing reflog iterators. This removes one
instance where we manually unmarshall the reflog file format.
Remove the now redundant ref_msg function.
Log messages for errors are changed slightly. We no longer print the file
name for the reflog, instead we refer to it as 'Log for ref refname'.
This might be a minor useability regression, but I don't really think so, since
experienced users would know where the log is anyway and inexperienced users
would not know what to do about/how to repair 'Log ... has gap ...' anyway.
Adapt the t1400 test to handle the change in log messages.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c| 208 +-
t/t1400-update-ref.sh | 4 +-
2 files changed, 105 insertions(+), 107 deletions(-)
diff --git a/refs.c b/refs.c
index 6898263..29eb7eb 100644
--- a/refs.c
+++ b/refs.c
@@ -2926,119 +2926,117 @@ int create_symref(const char *ref_target, const char
*refs_heads_master,
return 0;
}
-static char *ref_msg(const char *line, const char *endp)
-{
- const char *ep;
- line += 82;
- ep = memchr(line, '\n', endp - line);
- if (!ep)
- ep = endp;
- return xmemdupz(line, ep - line);
+struct read_ref_at_cb {
+ const char *refname;
+ unsigned long at_time;
+ int cnt;
+ int reccnt;
+ unsigned char *sha1;
+ int found_it;
+
+ unsigned char osha1[20];
+ unsigned char nsha1[20];
+ int tz;
+ unsigned long date;
+ char **msg;
+ unsigned long *cutoff_time;
+ int *cutoff_tz;
+ int *cutoff_cnt;
+};
+
+static int read_ref_at_ent(unsigned char *osha1, unsigned char *nsha1,
+ const char *email, unsigned long timestamp, int tz,
+ const char *message, void *cb_data)
+{
+ struct read_ref_at_cb *cb = cb_data;
+
+ cb-reccnt++;
+ cb-tz = tz;
+ cb-date = timestamp;
+
+ if (timestamp = cb-at_time || cb-cnt == 0) {
+ if (cb-msg)
+ *cb-msg = xstrdup(message);
+ if (cb-cutoff_time)
+ *cb-cutoff_time = timestamp;
+ if (cb-cutoff_tz)
+ *cb-cutoff_tz = tz;
+ if (cb-cutoff_cnt)
+ *cb-cutoff_cnt = cb-reccnt - 1;
+ /*
+* we have not yet updated cb-[n|o]sha1 so they still
+* hold the values for the previous record.
+*/
+ if (!is_null_sha1(cb-osha1)) {
+ hashcpy(cb-sha1, nsha1);
+ if (hashcmp(cb-osha1, nsha1))
+ warning(Log for ref %s has gap after %s.,
+ cb-refname, show_date(cb-date,
cb-tz, DATE_RFC2822));
+ }
+ else if (cb-date == cb-at_time)
+ hashcpy(cb-sha1, nsha1);
+ else if (hashcmp(nsha1, cb-sha1))
+ warning(Log for ref %s unexpectedly ended on %s.,
+ cb-refname, show_date(cb-date, cb-tz,
+ DATE_RFC2822));
+ hashcpy(cb-osha1, osha1);
+ hashcpy(cb-nsha1, nsha1);
+ cb-found_it = 1;
+ return 1;
+ }
+ hashcpy(cb-osha1, osha1);
+ hashcpy(cb-nsha1, nsha1);
+ if (cb-cnt 0)
+ cb-cnt--;
+ return 0;
+}
+
+static int read_ref_at_ent_oldest(unsigned char *osha1, unsigned char *nsha1,
+ const char *email, unsigned long timestamp,
+ int tz, const char *message, void *cb_data)
+{
+ struct read_ref_at_cb *cb = cb_data;
+
+ if (cb-msg)
+ *cb-msg = xstrdup(message);
+ if (cb-cutoff_time)
+ *cb-cutoff_time = timestamp;
+ if (cb-cutoff_tz)
+ *cb-cutoff_tz = tz;
+ if (cb-cutoff_cnt)
+ *cb-cutoff_cnt = cb-reccnt;
+ hashcpy(cb-sha1, osha1);
+ if (is_null_sha1(cb-sha1))
+ hashcpy(cb-sha1, nsha1);
+ /* We just want the first entry */
+ return 1;
}
int read_ref_at(const char *refname, unsigned long at_time, int cnt,
unsigned char *sha1, char **msg,
unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt)
{
- const char *logfile, *logdata, *logend, *rec, *lastgt, *lastrec;
- char *tz_c;
- int logfd, tz, reccnt = 0;
- struct stat st;
- unsigned long date;
- unsigned char logged_sha1[20];
- void *log_mapped;
- size_t mapsz;
+ struct read_ref_at_cb cb;
- logfile = git_path(logs/%s, refname);
- logfd = open(logfile, O_RDONLY, 0);
- if (logfd 0)
- die_errno(Unable
Re: [PATCH] refs.c: change read_ref_at to use the reflog iterators
On Fri, May 30, 2014 at 2:59 PM, Junio C Hamano gits...@pobox.com wrote: Ronnie Sahlberg sahlb...@google.com writes: read_ref_at has its own parsing of the reflog file for no really good reason so lets change this to use the existing reflog iterators. This removes one instance where we manually unmarshall the reflog file format. Log messages for errors are changed slightly. We no longer print the file name for the reflog, instead we refer to it as 'Log for ref refname'. This might be a minor useability regression, but I don't really think so, since experienced users would know where the log is anyway and inexperienced users would not know what to do about/how to repair 'Log ... has gap ...' anyway. Adapt the t1400 test to handle the cahnge in log messages. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c| 217 -- t/t1400-update-ref.sh | 4 +- 2 files changed, 122 insertions(+), 99 deletions(-) After reading the log message we are removing one redundant implementation, I would have expected that such a change would remove a lot more lines than it would add. Seeing the diffstat, I have to wonder if the fact that we need more lines to reuse the API is an indication that the reflog iterator API is overly cumbersome to use, perhaps requiring too much boilerplate or something. Yeah. We simplify the code and make it reuse existing unmarshallers and make it easier to read, and the line count goes up :-( Most of the extra code is the due to the structure to hold all the data we need in the callbacks and is a bit less compact. That said, I think the new code is a little easier to read. The biggest value is that we reduce the number of reflog unmarshallers by one which will save work when we start storing reflogs in a different type of backend. The update in the error message may be a side-effect, but I think it is a change in the good direction. The update in error message is also to prepare for the possibility that we might get a different type of ref and reflog store in the future. So that we only generate log messages that refer to filenames in those places where we are actually accessing files directly. Thanks. I will resend the patch later with Eric's suggestions. ronnie sahlberg -- 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] refs.c: change read_ref_at to use the reflog iterators
read_ref_at has its own parsing of the reflog file for no really good reason
so lets change this to use the existing reflog iterators. This removes one
instance where we manually unmarshall the reflog file format.
Log messages for errors are changed slightly. We no longer print the file
name for the reflog, instead we refer to it as 'Log for ref refname'.
This might be a minor useability regression, but I don't really think so, since
experienced users would know where the log is anyway and inexperienced users
would not know what to do about/how to repair 'Log ... has gap ...' anyway.
Adapt the t1400 test to handle the change in log messages.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c| 202 ++
t/t1400-update-ref.sh | 4 +-
2 files changed, 107 insertions(+), 99 deletions(-)
diff --git a/refs.c b/refs.c
index 6898263..b45bb2f 100644
--- a/refs.c
+++ b/refs.c
@@ -2936,109 +2936,117 @@ static char *ref_msg(const char *line, const char
*endp)
return xmemdupz(line, ep - line);
}
+struct read_ref_at_cb {
+ const char *refname;
+ unsigned long at_time;
+ int cnt;
+ int reccnt;
+ unsigned char *sha1;
+ int found_it;
+
+ char osha1[20];
+ char nsha1[20];
+ int tz;
+ unsigned long date;
+ char **msg;
+ unsigned long *cutoff_time;
+ int *cutoff_tz;
+ int *cutoff_cnt;
+};
+
+static int read_ref_at_ent(unsigned char *osha1, unsigned char *nsha1,
+ const char *email, unsigned long timestamp, int tz,
+ const char *message, void *cb_data)
+{
+ struct read_ref_at_cb *cb = cb_data;
+
+ cb-reccnt++;
+ cb-tz = tz;
+ cb-date = timestamp;
+
+ if (timestamp = cb-at_time || cb-cnt == 0) {
+ if (cb-msg)
+ *cb-msg = xstrdup(message);
+ if (cb-cutoff_time)
+ *cb-cutoff_time = timestamp;
+ if (cb-cutoff_tz)
+ *cb-cutoff_tz = tz;
+ if (cb-cutoff_cnt)
+ *cb-cutoff_cnt = cb-reccnt - 1;
+ /*
+* we have not yet updated cb-[n|o]sha1 so they still
+* hold the values for the previous record.
+*/
+ if (!is_null_sha1(cb-osha1)) {
+ hashcpy(cb-sha1, nsha1);
+ if (hashcmp(cb-osha1, nsha1))
+ warning(Log for ref %s has gap after %s.,
+ cb-refname, show_date(cb-date,
cb-tz, DATE_RFC2822));
+ }
+ else if (cb-date == cb-at_time)
+ hashcpy(cb-sha1, nsha1);
+ else if (hashcmp(nsha1, cb-sha1))
+ warning(Log for ref %s unexpectedly ended on %s.,
+ cb-refname, show_date(cb-date, cb-tz,
+ DATE_RFC2822));
+ hashcpy(cb-osha1, osha1);
+ hashcpy(cb-nsha1, nsha1);
+ cb-found_it = 1;
+ return 1;
+ }
+ hashcpy(cb-osha1, osha1);
+ hashcpy(cb-nsha1, nsha1);
+ if (cb-cnt 0)
+ cb-cnt--;
+ return 0;
+}
+
+static int read_ref_at_ent_oldest(unsigned char *osha1, unsigned char *nsha1,
+ const char *email, unsigned long timestamp,
+ int tz, const char *message, void *cb_data)
+{
+ struct read_ref_at_cb *cb = cb_data;
+
+ if (cb-msg)
+ *cb-msg = xstrdup(message);
+ if (cb-cutoff_time)
+ *cb-cutoff_time = timestamp;
+ if (cb-cutoff_tz)
+ *cb-cutoff_tz = tz;
+ if (cb-cutoff_cnt)
+ *cb-cutoff_cnt = cb-reccnt;
+ hashcpy(cb-sha1, osha1);
+ if (is_null_sha1(cb-sha1))
+ hashcpy(cb-sha1, nsha1);
+ /* We just want the first entry */
+ return 1;
+}
+
int read_ref_at(const char *refname, unsigned long at_time, int cnt,
unsigned char *sha1, char **msg,
unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt)
{
- const char *logfile, *logdata, *logend, *rec, *lastgt, *lastrec;
- char *tz_c;
- int logfd, tz, reccnt = 0;
- struct stat st;
- unsigned long date;
- unsigned char logged_sha1[20];
- void *log_mapped;
- size_t mapsz;
+ struct read_ref_at_cb cb;
- logfile = git_path(logs/%s, refname);
- logfd = open(logfile, O_RDONLY, 0);
- if (logfd 0)
- die_errno(Unable to read log '%s', logfile);
- fstat(logfd, st);
- if (!st.st_size)
- die(Log %s is empty., logfile);
- mapsz = xsize_t(st.st_size);
- log_mapped = xmmap(NULL, mapsz, PROT_READ, MAP_PRIVATE, logfd, 0);
- logdata = log_mapped;
- close(logfd);
Re: [PATCH] refs.c: change read_ref_at to use the reflog iterators
Ronnie Sahlberg sahlb...@google.com writes:
read_ref_at has its own parsing of the reflog file for no really good reason
so lets change this to use the existing reflog iterators. This removes one
instance where we manually unmarshall the reflog file format.
Log messages for errors are changed slightly. We no longer print the file
name for the reflog, instead we refer to it as 'Log for ref refname'.
This might be a minor useability regression, but I don't really think so,
since
experienced users would know where the log is anyway and inexperienced users
would not know what to do about/how to repair 'Log ... has gap ...' anyway.
Adapt the t1400 test to handle the change in log messages.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c| 202
++
t/t1400-update-ref.sh | 4 +-
2 files changed, 107 insertions(+), 99 deletions(-)
diff --git a/refs.c b/refs.c
index 6898263..b45bb2f 100644
--- a/refs.c
+++ b/refs.c
@@ -2936,109 +2936,117 @@ static char *ref_msg(const char *line, const char
*endp)
return xmemdupz(line, ep - line);
}
If I am not mistaken, this function will become unused with this
rewrite. Let's drop it and justify it in the log message.
+struct read_ref_at_cb {
+ const char *refname;
+ unsigned long at_time;
+ int cnt;
+ int reccnt;
+ unsigned char *sha1;
+ int found_it;
+
+ char osha1[20];
+ char nsha1[20];
These should be unsigned char, shouldn't they?
+ for_each_reflog_ent_reverse(refname, read_ref_at_ent, cb);
+
+ if (!cb.reccnt)
+ die(Log for %s is empty., refname);
+ if (cb.found_it)
+ return 0;
+
+ for_each_reflog_ent(refname, read_ref_at_ent_oldest, cb);
Hmph.
We have already scanned the same reflog in the backward direction in
full. Do we need to make another call just to pick up the entry at
the beginning? Is this because the callback is not told that it is
fed the last entry (in other words, if there is some clue that this
is the last call from for-each-reflog-ent-reverse to the callback,
the callback could record the value it received in its cb-data)?
--
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] refs.c: change read_ref_at to use the reflog iterators
read_ref_at has its own parsing of the reflog file for no really good reason
so lets change this to use the existing reflog iterators. This removes one
instance where we manually unmarshall the reflog file format.
Log messages for errors are changed slightly. We no longer print the file
name for the reflog, instead we refer to it as 'Log for ref refname'.
This might be a minor useability regression, but I don't really think so, since
experienced users would know where the log is anyway and inexperienced users
would not know what to do about/how to repair 'Log ... has gap ...' anyway.
Adapt the t1400 test to handle the cahnge in log messages.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c| 217 --
t/t1400-update-ref.sh | 4 +-
2 files changed, 122 insertions(+), 99 deletions(-)
diff --git a/refs.c b/refs.c
index 6898263..99d4832 100644
--- a/refs.c
+++ b/refs.c
@@ -2936,109 +2936,132 @@ static char *ref_msg(const char *line, const char
*endp)
return xmemdupz(line, ep - line);
}
+struct read_ref_at_cb {
+ const char *refname;
+ unsigned long at_time;
+ int cnt;
+ int reccnt;
+ unsigned char *sha1;
+ int found_it;
+
+ char osha1[20];
+ char nsha1[20];
+ int tz;
+ unsigned long date;
+ char **msg;
+ unsigned long *cutoff_time;
+ int *cutoff_tz;
+ int *cutoff_cnt;
+};
+
+static int read_ref_at_ent(unsigned char *osha1, unsigned char *nsha1,
+ const char *email, unsigned long timestamp, int tz,
+ const char *message, void *cb_data)
+{
+ struct read_ref_at_cb *cb = cb_data;
+
+ cb-reccnt++;
+ cb-tz = tz;
+ cb-date = timestamp;
+
+ if (timestamp = cb-at_time || cb-cnt == 0) {
+ if (cb-msg)
+ *cb-msg = xstrdup(message);
+ if (cb-cutoff_time)
+ *cb-cutoff_time = timestamp;
+ if (cb-cutoff_tz)
+ *cb-cutoff_tz = tz;
+ if (cb-cutoff_cnt)
+ *cb-cutoff_cnt = cb-reccnt - 1;
+
+ /*
+* we have not yet updated cb-[n|o]sha1 so they still
+* hold the values for the previous record.
+*/
+ if (!is_null_sha1(cb-osha1)) {
+ hashcpy(cb-sha1, nsha1);
+ if (hashcmp(cb-osha1, nsha1))
+ warning(Log for ref %s has gap after %s.,
+ cb-refname, show_date(cb-date,
cb-tz, DATE_RFC2822));
+ }
+ else if (cb-date == cb-at_time)
+ hashcpy(cb-sha1, nsha1);
+ else
+ if (hashcmp(nsha1, cb-sha1))
+ warning(Log for ref %s unexpectedly ended on
%s.,
+ cb-refname, show_date(cb-date, cb-tz,
+ DATE_RFC2822));
+
+ /*
+* return 1. Not to signal an error but to break the loop
+* since we have found the entry we want.
+*/
+ hashcpy(cb-osha1, osha1);
+ hashcpy(cb-nsha1, nsha1);
+ cb-found_it = 1;
+ return 1;
+ }
+
+ hashcpy(cb-osha1, osha1);
+ hashcpy(cb-nsha1, nsha1);
+ if (cb-cnt 0)
+ cb-cnt--;
+
+ return 0;
+}
+
+static int read_ref_at_ent_oldest(unsigned char *osha1, unsigned char *nsha1,
+ const char *email, unsigned long timestamp,
+ int tz, const char *message, void *cb_data)
+{
+ struct read_ref_at_cb *cb = cb_data;
+
+ if (cb-msg)
+ *cb-msg = xstrdup(message);
+ if (cb-cutoff_time)
+ *cb-cutoff_time = timestamp;
+ if (cb-cutoff_tz)
+ *cb-cutoff_tz = tz;
+ if (cb-cutoff_cnt)
+ *cb-cutoff_cnt = cb-reccnt - 1;
+
+ hashcpy(cb-sha1, osha1);
+ if (is_null_sha1(cb-sha1))
+ hashcpy(cb-sha1, nsha1);
+
+ /*
+* return 1. Not to signal an error but to break the loop
+* since we have found the entry we want.
+*/
+ return 1;
+}
+
int read_ref_at(const char *refname, unsigned long at_time, int cnt,
unsigned char *sha1, char **msg,
unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt)
{
- const char *logfile, *logdata, *logend, *rec, *lastgt, *lastrec;
- char *tz_c;
- int logfd, tz, reccnt = 0;
- struct stat st;
- unsigned long date;
- unsigned char logged_sha1[20];
- void *log_mapped;
- size_t mapsz;
+ struct read_ref_at_cb cb;
- logfile = git_path(logs/%s, refname);
- logfd = open(logfile, O_RDONLY, 0);
- if
Re: [PATCH] refs.c: change read_ref_at to use the reflog iterators
Ronnie Sahlberg sahlb...@google.com writes:
read_ref_at has its own parsing of the reflog file for no really good reason
so lets change this to use the existing reflog iterators. This removes one
instance where we manually unmarshall the reflog file format.
Log messages for errors are changed slightly. We no longer print the file
name for the reflog, instead we refer to it as 'Log for ref refname'.
This might be a minor useability regression, but I don't really think so,
since
experienced users would know where the log is anyway and inexperienced users
would not know what to do about/how to repair 'Log ... has gap ...' anyway.
Adapt the t1400 test to handle the cahnge in log messages.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c| 217
--
t/t1400-update-ref.sh | 4 +-
2 files changed, 122 insertions(+), 99 deletions(-)
After reading the log message we are removing one redundant
implementation, I would have expected that such a change would
remove a lot more lines than it would add. Seeing the diffstat, I
have to wonder if the fact that we need more lines to reuse the API
is an indication that the reflog iterator API is overly cumbersome
to use, perhaps requiring too much boilerplate or something.
The update in the error message may be a side-effect, but I think it
is a change in the good direction.
Thanks.
diff --git a/refs.c b/refs.c
index 6898263..99d4832 100644
--- a/refs.c
+++ b/refs.c
@@ -2936,109 +2936,132 @@ static char *ref_msg(const char *line, const char
*endp)
return xmemdupz(line, ep - line);
}
+struct read_ref_at_cb {
+ const char *refname;
+ unsigned long at_time;
+ int cnt;
+ int reccnt;
+ unsigned char *sha1;
+ int found_it;
+
+ char osha1[20];
+ char nsha1[20];
+ int tz;
+ unsigned long date;
+ char **msg;
+ unsigned long *cutoff_time;
+ int *cutoff_tz;
+ int *cutoff_cnt;
+};
+
+static int read_ref_at_ent(unsigned char *osha1, unsigned char *nsha1,
+ const char *email, unsigned long timestamp, int tz,
+ const char *message, void *cb_data)
+{
+ struct read_ref_at_cb *cb = cb_data;
+
+ cb-reccnt++;
+ cb-tz = tz;
+ cb-date = timestamp;
+
+ if (timestamp = cb-at_time || cb-cnt == 0) {
+ if (cb-msg)
+ *cb-msg = xstrdup(message);
+ if (cb-cutoff_time)
+ *cb-cutoff_time = timestamp;
+ if (cb-cutoff_tz)
+ *cb-cutoff_tz = tz;
+ if (cb-cutoff_cnt)
+ *cb-cutoff_cnt = cb-reccnt - 1;
+
+ /*
+ * we have not yet updated cb-[n|o]sha1 so they still
+ * hold the values for the previous record.
+ */
+ if (!is_null_sha1(cb-osha1)) {
+ hashcpy(cb-sha1, nsha1);
+ if (hashcmp(cb-osha1, nsha1))
+ warning(Log for ref %s has gap after %s.,
+ cb-refname, show_date(cb-date,
cb-tz, DATE_RFC2822));
+ }
+ else if (cb-date == cb-at_time)
+ hashcpy(cb-sha1, nsha1);
+ else
+ if (hashcmp(nsha1, cb-sha1))
+ warning(Log for ref %s unexpectedly ended on
%s.,
+ cb-refname, show_date(cb-date, cb-tz,
+DATE_RFC2822));
+
+ /*
+ * return 1. Not to signal an error but to break the loop
+ * since we have found the entry we want.
+ */
+ hashcpy(cb-osha1, osha1);
+ hashcpy(cb-nsha1, nsha1);
+ cb-found_it = 1;
+ return 1;
+ }
+
+ hashcpy(cb-osha1, osha1);
+ hashcpy(cb-nsha1, nsha1);
+ if (cb-cnt 0)
+ cb-cnt--;
+
+ return 0;
+}
+
+static int read_ref_at_ent_oldest(unsigned char *osha1, unsigned char *nsha1,
+ const char *email, unsigned long timestamp,
+ int tz, const char *message, void *cb_data)
+{
+ struct read_ref_at_cb *cb = cb_data;
+
+ if (cb-msg)
+ *cb-msg = xstrdup(message);
+ if (cb-cutoff_time)
+ *cb-cutoff_time = timestamp;
+ if (cb-cutoff_tz)
+ *cb-cutoff_tz = tz;
+ if (cb-cutoff_cnt)
+ *cb-cutoff_cnt = cb-reccnt - 1;
+
+ hashcpy(cb-sha1, osha1);
+ if (is_null_sha1(cb-sha1))
+ hashcpy(cb-sha1, nsha1);
+
+ /*
+ * return 1. Not to signal an error but to break the loop
+ * since we have found the entry we want.
+ */
+ return 1;
+}
+
int read_ref_at(const char *refname, unsigned long at_time, int cnt,
Re: [PATCH] refs.c: change read_ref_at to use the reflog iterators
On Fri, May 30, 2014 at 3:51 PM, Ronnie Sahlberg sahlb...@google.com wrote:
read_ref_at has its own parsing of the reflog file for no really good reason
so lets change this to use the existing reflog iterators. This removes one
instance where we manually unmarshall the reflog file format.
Log messages for errors are changed slightly. We no longer print the file
name for the reflog, instead we refer to it as 'Log for ref refname'.
This might be a minor useability regression, but I don't really think so,
since
experienced users would know where the log is anyway and inexperienced users
would not know what to do about/how to repair 'Log ... has gap ...' anyway.
Adapt the t1400 test to handle the cahnge in log messages.
s/cahnge/change/
More below.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
diff --git a/refs.c b/refs.c
index 6898263..99d4832 100644
--- a/refs.c
+++ b/refs.c
@@ -2936,109 +2936,132 @@ static char *ref_msg(const char *line, const char
*endp)
return xmemdupz(line, ep - line);
}
+static int read_ref_at_ent(unsigned char *osha1, unsigned char *nsha1,
+ const char *email, unsigned long timestamp, int tz,
+ const char *message, void *cb_data)
+{
+ struct read_ref_at_cb *cb = cb_data;
+
+ cb-reccnt++;
+ cb-tz = tz;
+ cb-date = timestamp;
+
+ if (timestamp = cb-at_time || cb-cnt == 0) {
+ if (cb-msg)
+ *cb-msg = xstrdup(message);
+ if (cb-cutoff_time)
+ *cb-cutoff_time = timestamp;
+ if (cb-cutoff_tz)
+ *cb-cutoff_tz = tz;
+ if (cb-cutoff_cnt)
+ *cb-cutoff_cnt = cb-reccnt - 1;
+
+ /*
+* we have not yet updated cb-[n|o]sha1 so they still
+* hold the values for the previous record.
+*/
+ if (!is_null_sha1(cb-osha1)) {
+ hashcpy(cb-sha1, nsha1);
+ if (hashcmp(cb-osha1, nsha1))
+ warning(Log for ref %s has gap after %s.,
+ cb-refname, show_date(cb-date,
cb-tz, DATE_RFC2822));
+ }
+ else if (cb-date == cb-at_time)
+ hashcpy(cb-sha1, nsha1);
+ else
+ if (hashcmp(nsha1, cb-sha1))
This could be an 'else if', allowing you to drop one level of indentation.
+ warning(Log for ref %s unexpectedly ended on
%s.,
+ cb-refname, show_date(cb-date,
cb-tz,
+ DATE_RFC2822));
+
+ /*
+* return 1. Not to signal an error but to break the loop
+* since we have found the entry we want.
+*/
+ hashcpy(cb-osha1, osha1);
+ hashcpy(cb-nsha1, nsha1);
+ cb-found_it = 1;
+ return 1;
+ }
+
+ hashcpy(cb-osha1, osha1);
+ hashcpy(cb-nsha1, nsha1);
+ if (cb-cnt 0)
+ cb-cnt--;
+
+ return 0;
+}
--
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
