Re: [PATCH 1/1] RFC: git: update to v2.19.0-rc0

2018-08-21 Thread Christian Hesse
John Keeping  on Tue, 2018/08/21 09:46:
> On Tue, Aug 21, 2018 at 09:21:14AM +0200, Christian Hesse wrote:
> > From: Christian Hesse 
> > 
> > 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).

This is related to git 2.19, see "Merge branch 'jk/banned-function'" [0].
But you are right this can go in in separate patches if we do it before
git 2.19 final.

I will look at your comments. Thanks!

[0] https://github.com/git/git/commit/e28daf222f51f137d9038a58812f2a89f414781e
-- 
main(a){char*c=/*Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/*Best regards my address:*/=0;b=c[a++];)
putchar(b-1/(/*Chriscc -ox -xc - && ./x*/b/42*2-3)*42);}


pgpWCdu9ObMDd.pgp
Description: OpenPGP digital signature
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit


Re: [PATCH 1/1] RFC: git: update to v2.19.0-rc0

2018-08-21 Thread John Keeping
On Tue, Aug 21, 2018 at 09:21:14AM +0200, Christian Hesse wrote:
> From: Christian Hesse 
> 
> 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 
> ---
>  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 = 
> -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 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/git b/git
> index 53f9a3e..7e8bfb0 16
> --- 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(, "\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 

[PATCH 1/1] RFC: git: update to v2.19.0-rc0

2018-08-21 Thread Christian Hesse
From: Christian Hesse 

Changelog to be writting... :)

Signed-off-by: Christian Hesse 
---
 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 = 
-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 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/git b/git
index 53f9a3e..7e8bfb0 16
--- 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);
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);
 
if (ident.date_begin)
*date = strtoul(ident.date_begin, NULL, 10);
@@ -130,7 +129,7 @@ struct commitinfo *cgit_parse_commit(struct commit *commit)
 {
const int sha1hex_len = 40;
struct commitinfo *ret;
-   const char *p = get_cached_commit_buffer(commit, NULL);
+   const char *p = get_cached_commit_buffer(the_repository, commit, NULL);
const char *t;
 
ret = xcalloc(1, sizeof(struct commitinfo));
diff --git a/shared.c b/shared.c
index 609bd2a..7560f5f 100644
--- a/shared.c
+++ b/shared.c
@@ -161,7 +161,7 @@ static struct refinfo *cgit_mk_refinfo(const char *refname, 
const struct object_
 
ref = xmalloc(sizeof (struct refinfo));
ref->refname = xstrdup(refname);
-   ref->object = parse_object(oid);
+   ref->object = parse_object(the_repository, oid);
switch (ref->object->type) {
case OBJ_TAG:
ref->tag = cgit_parse_tag((struct tag *)ref->object);
diff --git a/ui-blame.c b/ui-blame.c
index 50d0580..6dc555f 100644
--- a/ui-blame.c
+++ b/ui-blame.c
@@ -278,7 +278,7 @@ void cgit_print_blame(void)
"Invalid revision name: %s", rev);
return;
}
-   commit = lookup_commit_reference();
+   commit = lookup_commit_reference(the_repository, );
if (!commit || parse_commit(commit)) {
cgit_print_error_page(404, "Not found",
"Invalid commit reference: %s", rev);
diff --git a/ui-blob.c b/ui-blob.c
index 7b6da2a..4b6b462 100644
--- a/ui-blob.c
+++ b/ui-blob.c
@@ -56,7 +56,7 @@ int cgit_ref_path_exists(const char *path, const char *ref, 
int file_only)
goto done;
if (oid_object_info(the_repository, , ) != OBJ_COMMIT)
goto done;
-   read_tree_recursive(lookup_commit_reference()->maybe_tree, "", 0, 
0, , walk_tree, _tree_ctx);
+   read_tree_recursive(lookup_commit_reference(the_repository, 
)->maybe_tree, "", 0, 0, , walk_tree, _tree_ctx);
 
 done:
free(path_items.match);
@@ -89,7 +89,7 @@ int cgit_print_file(char *path, const char *head, int 
file_only)
return -1;
type = oid_object_info(the_repository, , );
if (type == OBJ_COMMIT) {
-   commit = lookup_commit_reference();
+   commit = lookup_commit_reference(the_repository, );
read_tree_recursive(commit->maybe_tree, "", 0, 0, , 
walk_tree, _tree_ctx);
if (!walk_tree_ctx.found_path)
return -1;
@@ -145,7 +145,7 @@ void cgit_print_blob(const char *hex, char *path, const 
char *head, int file_onl
type = oid_object_info(the_repository, , );