receive-pack.c: consolidate find header logic

There are two functions that have very similar logic of finding a header
value. find_commit_header, and find_header. We can conslidate the logic
by introducing a new function find_header_mem, which is equivalent to
find_commit_header except it takes a len parameter that determines how
many bytes will be read. find_commit_header and find_header can then both
call find_header_mem.

This reduces duplicate logic, as the logic for finding header values
can now all live in one place.

Signed-off-by: John Cai <johncai86@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 4f92e6f059dcb3f6668ca9973dfee9f149ffbb27..37f77b96005f3ecb3d205d22bc90b4261efe2bd7 100644 (file)
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -581,32 +581,19 @@ static char *prepare_push_cert_nonce(const char *path, timestamp_t stamp)
        return strbuf_detach(&buf, NULL);
 }
 
-/*
- * NEEDSWORK: reuse find_commit_header() from jk/commit-author-parsing
- * after dropping "_commit" from its name and possibly moving it out
- * of commit.c
- */
 static char *find_header(const char *msg, size_t len, const char *key,
                         const char **next_line)
 {
-       int key_len = strlen(key);
-       const char *line = msg;
-
-       while (line && line < msg + len) {
-               const char *eol = strchrnul(line, '\n');
-
-               if ((msg + len <= eol) || line == eol)
-                       return NULL;
-               if (line + key_len < eol &&
-                   !memcmp(line, key, key_len) && line[key_len] == ' ') {
-                       int offset = key_len + 1;
-                       if (next_line)
-                               *next_line = *eol ? eol + 1 : eol;
-                       return xmemdupz(line + offset, (eol - line) - offset);
-               }
-               line = *eol ? eol + 1 : NULL;
-       }
-       return NULL;
+       size_t out_len;
+       const char *val = find_header_mem(msg, len, key, &out_len);
+
+       if (!val)
+               return NULL;
+
+       if (next_line)
+               *next_line = val + out_len + 1;
+
+       return xmemdupz(val, out_len);
 }
 
 /*

diff --git a/commit.c b/commit.c
index a348f085b2b853d2f14d6848f3ff57edf85601b8..28391c3468dc77b1b0f417c0eca9991273757be6 100644 (file)
--- a/commit.c
+++ b/commit.c
@@ -1631,12 +1631,20 @@ struct commit_list **commit_list_append(struct commit *commit,
        return &new_commit->next;
 }
 
-const char *find_commit_header(const char *msg, const char *key, size_t *out_len)
+const char *find_header_mem(const char *msg, size_t len,
+                       const char *key, size_t *out_len)
 {
        int key_len = strlen(key);
        const char *line = msg;
 
-       while (line) {
+       /*
+        * NEEDSWORK: It's possible for strchrnul() to scan beyond the range
+        * given by len. However, current callers are safe because they compute
+        * len by scanning a NUL-terminated block of memory starting at msg.
+        * Nonetheless, it would be better to ensure the function does not look
+        * at msg beyond the len provided by the caller.
+        */
+       while (line && line < msg + len) {
                const char *eol = strchrnul(line, '\n');
 
                if (line == eol)
@@ -1653,6 +1661,10 @@ const char *find_commit_header(const char *msg, const char *key, size_t *out_len
        return NULL;
 }
 
+const char *find_commit_header(const char *msg, const char *key, size_t *out_len)
+{
+       return find_header_mem(msg, strlen(msg), key, out_len);
+}
 /*
  * Inspect the given string and determine the true "end" of the log message, in
  * order to find where to put a new Signed-off-by trailer.  Ignored are

diff --git a/commit.h b/commit.h
index 3ea32766bcb00ada1c55f948ab6cc503b04c72a7..38cc5426615fdf1efd5b407e7507991db45100c2 100644 (file)
--- a/commit.h
+++ b/commit.h
@@ -290,12 +290,17 @@ void free_commit_extra_headers(struct commit_extra_header *extra);
 
 /*
  * Search the commit object contents given by "msg" for the header "key".
+ * Reads up to "len" bytes of "msg".
  * Returns a pointer to the start of the header contents, or NULL. The length
  * of the header, up to the first newline, is returned via out_len.
  *
  * Note that some headers (like mergetag) may be multi-line. It is the caller's
  * responsibility to parse further in this case!
  */
+const char *find_header_mem(const char *msg, size_t len,
+                       const char *key,
+                       size_t *out_len);
+
 const char *find_commit_header(const char *msg, const char *key,
                               size_t *out_len);