[PATCH 1/1] RFC: git: update to v2.19.0-rc0
John Keeping
john at keeping.me.uk
Tue Aug 21 10:46:50 CEST 2018
On Tue, Aug 21, 2018 at 09:21:14AM +0200, Christian Hesse wrote:
> From: Christian Hesse <mail at eworm.de>
>
> Changelog to be writting... :)
More comments below, but it looks like there's a lot of unrelated
cleanups here. I think only the the_repository parameter addition is
required for Git 2.19.
The other changes mostly look good, but I think they can be split out
into a separate series (which can probably land before Git 2.19 final is
released).
> Signed-off-by: Christian Hesse <mail at eworm.de>
> ---
> Makefile | 4 ++--
> cgit.h | 1 +
> git | 2 +-
> parsing.c | 7 +++----
> shared.c | 2 +-
> ui-blame.c | 2 +-
> ui-blob.c | 6 +++---
> ui-clone.c | 4 ++--
> ui-commit.c | 4 ++--
> ui-diff.c | 4 ++--
> ui-log.c | 4 ++--
> ui-patch.c | 11 +++++++----
> ui-plain.c | 2 +-
> ui-shared.c | 6 +++---
> ui-snapshot.c | 4 ++--
> ui-ssdiff.c | 9 +++++----
> ui-tag.c | 4 ++--
> ui-tree.c | 4 ++--
> 18 files changed, 42 insertions(+), 38 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 05ea71f..d67c8c6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -14,8 +14,8 @@ htmldir = $(docdir)
> pdfdir = $(docdir)
> mandir = $(prefix)/share/man
> SHA1_HEADER = <openssl/sha.h>
> -GIT_VER = 2.18.0
> -GIT_URL = https://www.kernel.org/pub/software/scm/git/git-$(GIT_VER).tar.gz
> +GIT_VER = 2.19.0.rc0
> +GIT_URL = https://www.kernel.org/pub/software/scm/git/testing/git-$(GIT_VER).tar.gz
> INSTALL = install
> COPYTREE = cp -r
> MAN5_TXT = $(wildcard *.5.txt)
> diff --git a/cgit.h b/cgit.h
> index 32dfd7a..bcc4fce 100644
> --- a/cgit.h
> +++ b/cgit.h
> @@ -8,6 +8,7 @@
> #include <cache.h>
> #include <grep.h>
> #include <object.h>
> +#include <object-store.h>
> #include <tree.h>
> #include <commit.h>
> #include <tag.h>
> diff --git a/git b/git
> index 53f9a3e..7e8bfb0 160000
> --- a/git
> +++ b/git
> @@ -1 +1 @@
> -Subproject commit 53f9a3e157dbbc901a02ac2c73346d375e24978c
> +Subproject commit 7e8bfb0412581daf8f3c89909f1d37844e8610dd
> diff --git a/parsing.c b/parsing.c
> index 12453c2..7b3980e 100644
> --- a/parsing.c
> +++ b/parsing.c
> @@ -63,8 +63,7 @@ static char *substr(const char *head, const char *tail)
> if (tail < head)
> return xstrdup("");
> buf = xmalloc(tail - head + 1);
> - strncpy(buf, head, tail - head);
> - buf[tail - head] = '\0';
> + strlcpy(buf, head, tail - head + 1);
Nice cleanup, but I don't think it is required for Git 2.19!
It's probably worth splitting this out for a separate patch that can
land before Git 2.19.
> return buf;
> }
>
> @@ -78,7 +77,7 @@ static void parse_user(const char *t, char **name, char **email, unsigned long *
>
> email_len = ident.mail_end - ident.mail_begin;
> *email = xmalloc(strlen("<") + email_len + strlen(">") + 1);
> - sprintf(*email, "<%.*s>", email_len, ident.mail_begin);
> + xsnprintf(*email, email_len + 3, "<%.*s>", email_len, ident.mail_begin);
Again, not related to Git 2.19. I'm not sure about this one, snprintf
isn't adding anything because we know exactly how big the buffer is.
However, I bet static analysis warns about sprintf here even though this
use is safe.
Maybe we should just use a strbuf?
>
> if (ident.date_begin)
> *date = strtoul(ident.date_begin, NULL, 10);
[snip the_repository conversion that all looks good]
> diff --git a/ui-log.c b/ui-log.c
> index d696e20..3bcb657 100644
> --- a/ui-log.c
> +++ b/ui-log.c
> @@ -67,7 +67,7 @@ void show_commit_decorations(struct commit *commit)
> while (deco) {
> struct object_id peeled;
> int is_annotated = 0;
> - strncpy(buf, prettify_refname(deco->name), sizeof(buf) - 1);
> + strlcpy(buf, prettify_refname(deco->name), sizeof(buf));
More cleanup to split out.
> switch(deco->type) {
> case DECORATION_NONE:
> /* If the git-core doesn't recognize it,
> @@ -234,7 +234,7 @@ static void print_commit(struct commit *commit, struct rev_info *revs)
> strbuf_add(&msgbuf, "\n\n", 2);
>
> /* Place wrap_symbol at position i in info->subject */
> - strcpy(info->subject + i, wrap_symbol);
> + strlcpy(info->subject + i, wrap_symbol, subject_len - i + 1);
More cleanup.
> }
> }
> cgit_commit_link(info->subject, NULL, NULL, ctx.qry.head,
> diff --git a/ui-patch.c b/ui-patch.c
> index 8007a11..efd7a34 100644
> --- a/ui-patch.c
> +++ b/ui-patch.c
> @@ -11,13 +11,16 @@
> #include "html.h"
> #include "ui-shared.h"
>
> +/* two SHA1 hashes with two dots in between and termination */
> +#define REV_RANGE_LEN 2 * 40 + 3
Should this use GIT_MAX_HEXSZ ?
> +
> void cgit_print_patch(const char *new_rev, const char *old_rev,
> const char *prefix)
> {
> struct rev_info rev;
> struct commit *commit;
> struct object_id new_rev_oid, old_rev_oid;
> - char rev_range[2 * 40 + 3];
> + char rev_range[REV_RANGE_LEN];
> const char *rev_argv[] = { NULL, "--reverse", "--format=email", rev_range, "--", prefix, NULL };
> int rev_argc = ARRAY_SIZE(rev_argv) - 1;
> char *patchname;
[snip more the_repository conversion.]
> @@ -60,7 +63,7 @@ void cgit_print_patch(const char *new_rev, const char *old_rev,
> if (is_null_oid(&old_rev_oid)) {
> memcpy(rev_range, oid_to_hex(&new_rev_oid), GIT_SHA1_HEXSZ + 1);
> } else {
> - sprintf(rev_range, "%s..%s", oid_to_hex(&old_rev_oid),
> + xsnprintf(rev_range, REV_RANGE_LEN, "%s..%s", oid_to_hex(&old_rev_oid),
> oid_to_hex(&new_rev_oid));
> }
>
> diff --git a/ui-plain.c b/ui-plain.c
> index ddb3e48..070c34b 100644
> --- a/ui-plain.c
> +++ b/ui-plain.c
> @@ -185,7 +185,7 @@ void cgit_print_plain(void)
> cgit_print_error_page(404, "Not found", "Not found");
> return;
> }
> - commit = lookup_commit_reference(&oid);
> + commit = lookup_commit_reference(the_repository, &oid);
> if (!commit || parse_commit(commit)) {
> cgit_print_error_page(404, "Not found", "Not found");
> return;
> diff --git a/ui-shared.c b/ui-shared.c
> index 739505a..f8b438b 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -1176,14 +1176,14 @@ void cgit_set_title_from_path(const char *path)
> continue;
> }
> strncat(new_title, &path[path_index + 1], path_last_end - path_index - 1);
> - strcat(new_title, "\\");
> + strncat(new_title, "\\", 2);
Does this add anything? (And I think n should be 1 here since "\\" is a
single character.)
> path_last_end = path_index;
> }
> }
> if (path_last_end)
> strncat(new_title, path, path_last_end);
>
> - strcat(new_title, " - ");
> - strcat(new_title, ctx.page.title);
> + strncat(new_title, " - ", 4);
> + strncat(new_title, ctx.page.title, sizeof(new_title) - strlen(new_title));
n should be "sizeof(new_title) - strlen(new_title) - 1" here shouldn't
it?
> ctx.page.title = new_title;
> }
[snip the rest]
I don't have any comments on the remainder of the patch that aren't just
repetitions of the above on more bits of code so I'm not going to type
them out :-)
More information about the CGit
mailing list