Re: [PATCH] Cast things properly to handle >2G files.

10 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] Cast things properly to handle >2G files.

From: Alfred M. Szmidt <hidden>
Date: 2016-06-15 22:46:57 

   > I cannot agree to the D-C-O in good faith, since it speaks of
   > open source licenses, a group of licenses that include non-free
   > software licenses, something which I cannot support.

   If you can't sign off on it, then Junio shouldn't take it, since
   you're basically saying that you cannot say that you own the
   copyrights or cannot license it under the appropriate copyright.

That is not what I am saying.  In either case, I already explained why
I cannot agree to the DCO above.  And you are right, Junio shouldn't
apply the patch if it goes against commit policies.

   Yes, it speaks of "open source licenses", but it says: "under the
   open source license indicated in the file", and "appropriate open
   source license".

That is a good point, almost none of the files in git specify the
license so it is impossible to know what one is actually agreeing too.

I would suggest the following update to the DCO, it makes it more
general both to free software hackers, and open source hackers alike.

  Developer's Certificate of Origin 1.2-draft

  By making a contribution to this project, I certify that:

  (a) The contribution was created in whole or in part by me and I
      have the right to submit it under the license indicated in the
      file; or

  (b) The contribution is based upon previous work that, to the best
      of my knowledge, is covered under an compatible license and I
      have the right under that license to submit that work with
      modifications, whether created in whole or in part by me, under
      the same license (unless I am permitted to submit under a
      different license), as indicated in the file; or

  (c) The contribution was provided directly to me by some other
      person who certified (a), (b) or (c) and I have not modified it.

  (d) I understand and agree that this project and the contribution
      are public and that a record of the contribution (including all
      personal information I submit with it, including my sign-off) is
      maintained indefinitely and may be redistributed consistent with
      this project or the license(s) involved.

Re: [PATCH] Cast things properly to handle >2G files.

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 22:46:57 


On Wed, 17 Jun 2009, Alfred M. Szmidt wrote:
I would suggest the following update to the DCO, it makes it more
general both to free software hackers, and open source hackers alike.
And I would suggest that Junio just not take patches from people who 
aren't able to read the existing DCO. It's not worth the pain. 

		Linus

Re: [PATCH] Cast things properly to handle >2G files.

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 22:46:57 


On Wed, 17 Jun 2009, Alfred M. Szmidt wrote:
That is a good point, almost none of the files in git specify the
license so it is impossible to know what one is actually agreeing too.
Right. Because that "COPYING" file is so hard to find.

You're making excuses. Just admit it.

		Linus

Fix big left-shifts of unsigned char

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 22:46:57 

Shifting 'unsigned char' or 'unsigned short' left can result in sign 
extension errors, since the C integer promotion rules means that the 
unsigned char/short will get implicitly promoted to a signed 'int' due to 
the shift (or due to other operations).

This normally doesn't matter, but if you shift things up sufficiently, it 
will now set the sign bit in 'int', and a subsequent cast to a bigger type 
(eg 'long' or 'unsigned long') will now sign-extend the value despite the 
original expression being unsigned.

One example of this would be something like

	unsigned long size;
	unsigned char c;

	size += c << 24;

where despite all the variables being unsigned, 'c << 24' ends up being a 
signed entity, and will get sign-extended when then doing the addition in 
an 'unsigned long' type.

Since git uses 'unsigned char' pointers extensively, we actually have this 
bug in a couple of places. 

I may have missed some, but this is the result of looking at

	git grep '[^0-9 	][ 	]*<<[ 	][a-z]' -- '*.c' '*.h'
	git grep '<<[   ]*24'

which catches at least the common byte cases (shifting variables by a 
variable amount, and shifting by 24 bits).

I also grepped for just 'unsigned char' variables in general, and 
converted the ones that most obviously ended up getting implicitly cast 
immediately anyway (eg hash_name(), encode_85()).

In addition to just avoiding 'unsigned char', this patch also tries to use 
a common idiom for the delta header size thing. We had three different 
variations on it: "& 0x7fUL" in one place (getting the sign extension 
right), and "& ~0x80" and "& 0x7f" in two other places (not getting it 
right). Apart from making them all just avoid using "unsigned char" at 
all, I also unified them to then use a simple "& 0x7f".

I considered making a sparse extension which warns about doing implicit 
casts from unsigned types to signed types, but it gets rather complex very 
quickly, so this is just a hack. 

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

This is _not_ tested in any way. And I got bored with getting rid of 
'unsigned char' variables, so I by no means did all of them, just the 
first few that caugth my grepping eye.

On Wed, 17 Jun 2009, Linus Torvalds wrote:
And I would suggest that Junio just not take patches from people who 
aren't able to read the existing DCO. It's not worth the pain. 

		Linus
 attr.c                   |    3 +--
 base85.c                 |    2 +-
 builtin-pack-objects.c   |    3 +--
 builtin-unpack-objects.c |    4 ++--
 delta.h                  |    5 ++---
 index-pack.c             |    6 +++---
 patch-delta.c            |    2 +-
 sha1_file.c              |    3 +--
 8 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/attr.c b/attr.c
index 98eb636..f8f6faa 100644
--- a/attr.c
+++ b/attr.c
@@ -35,8 +35,7 @@ static struct git_attr *(git_attr_hash[HASHSIZE]);
 
 static unsigned hash_name(const char *name, int namelen)
 {
-	unsigned val = 0;
-	unsigned char c;
+	unsigned val = 0, c;
 
 	while (namelen--) {
 		c = *name++;
diff --git a/base85.c b/base85.c
index b88270f..b417a15 100644
--- a/base85.c
+++ b/base85.c
@@ -91,7 +91,7 @@ void encode_85(char *buf, const unsigned char *data, int bytes)
 		unsigned acc = 0;
 		int cnt;
 		for (cnt = 24; cnt >= 0; cnt -= 8) {
-			int ch = *data++;
+			unsigned ch = *data++;
 			acc |= ch << cnt;
 			if (--bytes == 0)
 				break;
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 9742b45..941cc2d 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -653,8 +653,7 @@ static void rehash_objects(void)
 
 static unsigned name_hash(const char *name)
 {
-	unsigned char c;
-	unsigned hash = 0;
+	unsigned c, hash = 0;
 
 	if (!name)
 		return 0;
diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c
index 9a77323..8e831be 100644
--- a/builtin-unpack-objects.c
+++ b/builtin-unpack-objects.c
@@ -422,8 +422,8 @@ static void unpack_delta_entry(enum object_type type, unsigned long delta_size,
 static void unpack_one(unsigned nr)
 {
 	unsigned shift;
-	unsigned char *pack, c;
-	unsigned long size;
+	unsigned char *pack;
+	unsigned long size, c;
 	enum object_type type;
 
 	obj_list[nr].offset = consumed_bytes;
diff --git a/delta.h b/delta.h
index 40ccf5a..b9d333d 100644
--- a/delta.h
+++ b/delta.h
@@ -90,12 +90,11 @@ static inline unsigned long get_delta_hdr_size(const unsigned char **datap,
 					       const unsigned char *top)
 {
 	const unsigned char *data = *datap;
-	unsigned char cmd;
-	unsigned long size = 0;
+	unsigned long cmd, size = 0;
 	int i = 0;
 	do {
 		cmd = *data++;
-		size |= (cmd & ~0x80) << i;
+		size |= (cmd & 0x7f) << i;
 		i += 7;
 	} while (cmd & 0x80 && data < top);
 	*datap = data;
diff --git a/index-pack.c b/index-pack.c
index 6e93ee6..0c92baf 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -293,8 +293,8 @@ static void *unpack_entry_data(unsigned long offset, unsigned long size)
 
 static void *unpack_raw_entry(struct object_entry *obj, union delta_base *delta_base)
 {
-	unsigned char *p, c;
-	unsigned long size;
+	unsigned char *p;
+	unsigned long size, c;
 	off_t base_offset;
 	unsigned shift;
 	void *data;
@@ -312,7 +312,7 @@ static void *unpack_raw_entry(struct object_entry *obj, union delta_base *delta_
 		p = fill(1);
 		c = *p;
 		use(1);
-		size += (c & 0x7fUL) << shift;
+		size += (c & 0x7f) << shift;
 		shift += 7;
 	}
 	obj->size = size;
diff --git a/patch-delta.c b/patch-delta.c
index ed9db81..ef748ce 100644
--- a/patch-delta.c
+++ b/patch-delta.c
@@ -44,7 +44,7 @@ void *patch_delta(const void *src_buf, unsigned long src_size,
 			if (cmd & 0x01) cp_off = *data++;
 			if (cmd & 0x02) cp_off |= (*data++ << 8);
 			if (cmd & 0x04) cp_off |= (*data++ << 16);
-			if (cmd & 0x08) cp_off |= (*data++ << 24);
+			if (cmd & 0x08) cp_off |= ((unsigned) *data++ << 24);
 			if (cmd & 0x10) cp_size = *data++;
 			if (cmd & 0x20) cp_size |= (*data++ << 8);
 			if (cmd & 0x40) cp_size |= (*data++ << 16);
diff --git a/sha1_file.c b/sha1_file.c
index e73cd4f..8f5fe62 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1162,8 +1162,7 @@ unsigned long unpack_object_header_buffer(const unsigned char *buf,
 		unsigned long len, enum object_type *type, unsigned long *sizep)
 {
 	unsigned shift;
-	unsigned char c;
-	unsigned long size;
+	unsigned long size, c;
 	unsigned long used = 0;
 
 	c = buf[used++];

Re: Fix big left-shifts of unsigned char

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:46:58 

Hi,

On Wed, 17 Jun 2009, Linus Torvalds wrote:

This is _not_ tested in any way. And I got bored with getting rid of 
'unsigned char' variables, so I by no means did all of them, just the 
first few that caugth my grepping eye.
I wonder if there is a mode of 'sparse' which could spot these buggers.

Ciao,
Dscho

Re: Fix big left-shifts of unsigned char

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 22:46:58 


On Thu, 18 Jun 2009, Johannes Schindelin wrote:
I wonder if there is a mode of 'sparse' which could spot these buggers.
Hmm. A very quick hack gets me (this is _after_ my patch):

	diffcore-delta.c:53:24: warning: Signed left-shift
	diffcore-delta.c:58:21: warning: Signed left-shift
	diffcore-delta.c:88:21: warning: Signed left-shift

which are all of the type "<< alloc_log", and not very interesting.

	read-cache.c:1155:17: warning: Signed left-shift

This one is "real", and I noticed it in my earlier greps, but we don't 
care (it's the CACHE_EXT_TREE/CACHE_EXT() matching, and it's all done in 
"int", and the only case we care about is a non-signed case anyway)

	imap-send.c:785:52: warning: Signed left-shift
	imap-send.c:1190:35: warning: Signed left-shift

These are both "real", but we're only working on "unsigned" so we don't 
really care.

	builtin-rev-list.c:201:21: warning: Signed left-shift

this is exp2i(), and it returns 'int', and wouldn't work for big left 
shifts anyway.

And just to see that my sparse logic actually worked, _without_ the patch 
I sent to fix left-shifts, I got these:

	base85.c:95:38: warning: Signed left-shift		(fixed)
	diffcore-delta.c:53:24: warning: Signed left-shift
	diffcore-delta.c:58:21: warning: Signed left-shift
	diffcore-delta.c:88:21: warning: Signed left-shift
	delta.h:98:42: warning: Signed left-shift		(fixed)
	delta.h:98:42: warning: Signed left-shift		(dup - inline)
	patch-delta.c:47:63: warning: Signed left-shift		(fixed)
	read-cache.c:1155:17: warning: Signed left-shift
	sha1_file.c:1179:39: warning: Signed left-shift		(fixed)
	delta.h:98:42: warning: Signed left-shift		(dup - inline)
	delta.h:98:42: warning: Signed left-shift		(dup - inline)
	imap-send.c:785:52: warning: Signed left-shift
	imap-send.c:1190:35: warning: Signed left-shift
	builtin-rev-list.c:201:21: warning: Signed left-shift
	builtin-unpack-objects.c:441:39: warning: Signed left-shift (fixed)

just to verify that my sparse checker actually found the ones the patch 
modified.

			Linus

Re: Fix big left-shifts of unsigned char

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:46:58 

Hi,

On Thu, 18 Jun 2009, Linus Torvalds wrote:

On Thu, 18 Jun 2009, Johannes Schindelin wrote:
quoted
I wonder if there is a mode of 'sparse' which could spot these 
buggers.
Hmm. A very quick hack gets me (this is _after_ my patch):

[snip thorough analysis]
Nice.

The quick hack is not going to be part of sparse.git, I take it?

Ciao,
Dscho

Re: Fix big left-shifts of unsigned char

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 22:46:58 


On Thu, 18 Jun 2009, Johannes Schindelin wrote:
The quick hack is not going to be part of sparse.git, I take it?
Not clear yet. I sent a couple of patches to the sparse list already to 
make things go better in general with running sparse on git. I can now do

	make "CC=cgcc -m64"

on my git tree, and get reasonable warnings. I'll play around with it 
that whole left-shift thing a bit more, but before I do that I'll post a 
patch for all the _other_ things sparse found in git.

		Linus

Fix various sparse warnings in the git source code

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 22:46:58 

There are a few remaining ones, but this fixes the trivial ones. It boils 
down to two main issues that sparse complains about:

 - warning: Using plain integer as NULL pointer

   Sparse doesn't like you using '0' instead of 'NULL'. For various good 
   reasons, not the least of which is just the visual confusion. A NULL 
   pointer is not an integer, and that whole "0 works as NULL" is a 
   historical accident and not very pretty.

   A few of these remain: zlib is a total mess, and Z_NULL is just a 0. 
   I didn't touch those.

 - warning: symbol 'xyz' was not declared. Should it be static?

   Sparse wwants to see declarations for any functions you export. A lack 
   fo a declaration tends to mean that you should either add one, or you 
   should mark the function 'static' to show that it's in file scope. 

   A few of these remain: I only did the ones that should obviously just 
   be made static.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

With this, and a few sparse fixes, git is _mostly_ sparse-clean.

On Thu, 18 Jun 2009, Linus Torvalds wrote:

On Thu, 18 Jun 2009, Johannes Schindelin wrote:
quoted
The quick hack is not going to be part of sparse.git, I take it?
Not clear yet. I sent a couple of patches to the sparse list already to 
make things go better in general with running sparse on git. I can now do

	make "CC=cgcc -m64"

on my git tree, and get reasonable warnings. I'll play around with it 
that whole left-shift thing a bit more, but before I do that I'll post a 
patch for all the _other_ things sparse found in git.

		Linus
 bisect.c                 |    4 ++--
 builtin-add.c            |    2 +-
 builtin-apply.c          |    2 +-
 builtin-clone.c          |    2 +-
 builtin-fsck.c           |    4 ++--
 builtin-help.c           |    2 +-
 builtin-log.c            |    4 ++--
 builtin-merge.c          |    4 ++--
 builtin-remote.c         |   16 ++++++++--------
 builtin-unpack-objects.c |    6 +++---
 connect.c                |    2 +-
 daemon.c                 |    2 +-
 imap-send.c              |    2 +-
 index-pack.c             |    2 +-
 mailmap.c                |    6 +++---
 merge-recursive.c        |    2 +-
 parse-options.c          |    2 +-
 quote.c                  |    4 ++--
 remote.c                 |    6 +++---
 test-parse-options.c     |    4 ++--
 20 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/bisect.c b/bisect.c
index 6fdff05..dbeb287 100644
--- a/bisect.c
+++ b/bisect.c
@@ -454,7 +454,7 @@ static int read_bisect_refs(void)
 	return for_each_ref_in("refs/bisect/", register_ref, NULL);
 }
 
-void read_bisect_paths(struct argv_array *array)
+static void read_bisect_paths(struct argv_array *array)
 {
 	struct strbuf str = STRBUF_INIT;
 	const char *filename = git_path("BISECT_NAMES");
@@ -780,7 +780,7 @@ static void handle_bad_merge_base(void)
 	exit(1);
 }
 
-void handle_skipped_merge_base(const unsigned char *mb)
+static void handle_skipped_merge_base(const unsigned char *mb)
 {
 	char *mb_hex = sha1_to_hex(mb);
 	char *bad_hex = sha1_to_hex(current_bad_sha1);
diff --git a/builtin-add.c b/builtin-add.c
index 566c313..ad8e562 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -160,7 +160,7 @@ int interactive_add(int argc, const char **argv, const char *prefix)
 	return status;
 }
 
-int edit_patch(int argc, const char **argv, const char *prefix)
+static int edit_patch(int argc, const char **argv, const char *prefix)
 {
 	char *file = xstrdup(git_path("ADD_EDIT.patch"));
 	const char *apply_argv[] = { "apply", "--recount", "--cached",
diff --git a/builtin-apply.c b/builtin-apply.c
index 94ba2bd..bb59567 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2614,7 +2614,7 @@ static int get_current_sha1(const char *path, unsigned char *sha1)
 static void build_fake_ancestor(struct patch *list, const char *filename)
 {
 	struct patch *patch;
-	struct index_state result = { 0 };
+	struct index_state result = { NULL };
 	int fd;
 
 	/* Once we start supporting the reverse patch, it may be
diff --git a/builtin-clone.c b/builtin-clone.c
index 5c46496..2ceacb7 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -141,7 +141,7 @@ static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)
 	if (is_bare) {
 		struct strbuf result = STRBUF_INIT;
 		strbuf_addf(&result, "%.*s.git", (int)(end - start), start);
-		dir = strbuf_detach(&result, 0);
+		dir = strbuf_detach(&result, NULL);
 	} else
 		dir = xstrndup(start, end - start);
 	/*
diff --git a/builtin-fsck.c b/builtin-fsck.c
index 7da706c..e077e72 100644
--- a/builtin-fsck.c
+++ b/builtin-fsck.c
@@ -104,7 +104,7 @@ static int mark_object(struct object *obj, int type, void *data)
 
 static void mark_object_reachable(struct object *obj)
 {
-	mark_object(obj, OBJ_ANY, 0);
+	mark_object(obj, OBJ_ANY, NULL);
 }
 
 static int traverse_one_object(struct object *obj, struct object *parent)
@@ -292,7 +292,7 @@ static int fsck_sha1(const unsigned char *sha1)
 		fprintf(stderr, "Checking %s %s\n",
 			typename(obj->type), sha1_to_hex(obj->sha1));
 
-	if (fsck_walk(obj, mark_used, 0))
+	if (fsck_walk(obj, mark_used, NULL))
 		objerror(obj, "broken links");
 	if (fsck_object(obj, check_strict, fsck_error_func))
 		return -1;
diff --git a/builtin-help.c b/builtin-help.c
index 6e53b23..e1eba77 100644
--- a/builtin-help.c
+++ b/builtin-help.c
@@ -394,7 +394,7 @@ static void get_html_page_path(struct strbuf *page_path, const char *page)
  * HTML.
  */
 #ifndef open_html
-void open_html(const char *path)
+static void open_html(const char *path)
 {
 	execl_git_cmd("web--browse", "-c", "help.browser", path, NULL);
 }
diff --git a/builtin-log.c b/builtin-log.c
index 0d34050..44f9a27 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -94,7 +94,7 @@ static void show_early_header(struct rev_info *rev, const char *stage, int nr)
 	printf("Final output: %d %s\n", nr, stage);
 }
 
-struct itimerval early_output_timer;
+static struct itimerval early_output_timer;
 
 static void log_show_early(struct rev_info *revs, struct commit_list *list)
 {
@@ -977,7 +977,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		strbuf_addch(&buf, '\n');
 	}
 
-	rev.extra_headers = strbuf_detach(&buf, 0);
+	rev.extra_headers = strbuf_detach(&buf, NULL);
 
 	if (start_number < 0)
 		start_number = 1;
diff --git a/builtin-merge.c b/builtin-merge.c
index 793f2f4..af9adab 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -370,7 +370,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
 
 	strbuf_addstr(&buf, "refs/heads/");
 	strbuf_addstr(&buf, remote);
-	resolve_ref(buf.buf, branch_head, 0, 0);
+	resolve_ref(buf.buf, branch_head, 0, NULL);
 
 	if (!hashcmp(remote_head->sha1, branch_head)) {
 		strbuf_addf(msg, "%s\t\tbranch '%s' of .\n",
@@ -409,7 +409,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
 		strbuf_addstr(&truname, "refs/heads/");
 		strbuf_addstr(&truname, remote);
 		strbuf_setlen(&truname, truname.len - len);
-		if (resolve_ref(truname.buf, buf_sha, 0, 0)) {
+		if (resolve_ref(truname.buf, buf_sha, 0, NULL)) {
 			strbuf_addf(msg,
 				    "%s\t\tbranch '%s'%s of .\n",
 				    sha1_to_hex(remote_head->sha1),
diff --git a/builtin-remote.c b/builtin-remote.c
index dfc0b9e..f73c657 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -740,7 +740,7 @@ static int rm(int argc, const char **argv)
 	return result;
 }
 
-void clear_push_info(void *util, const char *string)
+static void clear_push_info(void *util, const char *string)
 {
 	struct push_info *info = util;
 	free(info->dest);
@@ -815,7 +815,7 @@ struct show_info {
 	int any_rebase;
 };
 
-int add_remote_to_show_info(struct string_list_item *item, void *cb_data)
+static int add_remote_to_show_info(struct string_list_item *item, void *cb_data)
 {
 	struct show_info *info = cb_data;
 	int n = strlen(item->string);
@@ -825,7 +825,7 @@ int add_remote_to_show_info(struct string_list_item *item, void *cb_data)
 	return 0;
 }
 
-int show_remote_info_item(struct string_list_item *item, void *cb_data)
+static int show_remote_info_item(struct string_list_item *item, void *cb_data)
 {
 	struct show_info *info = cb_data;
 	struct ref_states *states = info->states;
@@ -852,7 +852,7 @@ int show_remote_info_item(struct string_list_item *item, void *cb_data)
 	return 0;
 }
 
-int add_local_to_show_info(struct string_list_item *branch_item, void *cb_data)
+static int add_local_to_show_info(struct string_list_item *branch_item, void *cb_data)
 {
 	struct show_info *show_info = cb_data;
 	struct ref_states *states = show_info->states;
@@ -874,7 +874,7 @@ int add_local_to_show_info(struct string_list_item *branch_item, void *cb_data)
 	return 0;
 }
 
-int show_local_info_item(struct string_list_item *item, void *cb_data)
+static int show_local_info_item(struct string_list_item *item, void *cb_data)
 {
 	struct show_info *show_info = cb_data;
 	struct branch_info *branch_info = item->util;
@@ -906,7 +906,7 @@ int show_local_info_item(struct string_list_item *item, void *cb_data)
 	return 0;
 }
 
-int add_push_to_show_info(struct string_list_item *push_item, void *cb_data)
+static int add_push_to_show_info(struct string_list_item *push_item, void *cb_data)
 {
 	struct show_info *show_info = cb_data;
 	struct push_info *push_info = push_item->util;
@@ -935,7 +935,7 @@ static int cmp_string_with_push(const void *va, const void *vb)
 	return cmp ? cmp : strcmp(a_push->dest, b_push->dest);
 }
 
-int show_push_info_item(struct string_list_item *item, void *cb_data)
+static int show_push_info_item(struct string_list_item *item, void *cb_data)
 {
 	struct show_info *show_info = cb_data;
 	struct push_info *push_info = item->util;
@@ -1187,7 +1187,7 @@ static int get_one_remote_for_update(struct remote *remote, void *priv)
 	return 0;
 }
 
-struct remote_group {
+static struct remote_group {
 	const char *name;
 	struct string_list *list;
 } remote_group;
diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c
index 9a77323..f8d597d 100644
--- a/builtin-unpack-objects.c
+++ b/builtin-unpack-objects.c
@@ -158,7 +158,7 @@ struct obj_info {
 #define FLAG_WRITTEN (1u<<21)
 
 static struct obj_info *obj_list;
-unsigned nr_objects;
+static unsigned nr_objects;
 
 /*
  * Called only from check_object() after it verified this object
@@ -200,7 +200,7 @@ static int check_object(struct object *obj, int type, void *data)
 
 	if (fsck_object(obj, 1, fsck_error_function))
 		die("Error in object");
-	if (!fsck_walk(obj, check_object, 0))
+	if (!fsck_walk(obj, check_object, NULL))
 		die("Error on reachable objects of %s", sha1_to_hex(obj->sha1));
 	write_cached_object(obj);
 	return 1;
@@ -210,7 +210,7 @@ static void write_rest(void)
 {
 	unsigned i;
 	for (i = 0; i < nr_objects; i++)
-		check_object(obj_list[i].obj, OBJ_ANY, 0);
+		check_object(obj_list[i].obj, OBJ_ANY, NULL);
 }
 
 static void added_object(unsigned nr, enum object_type type,
diff --git a/connect.c b/connect.c
index 0ce941e..76e5427 100644
--- a/connect.c
+++ b/connect.c
@@ -464,7 +464,7 @@ static void git_proxy_connect(int fd[2], char *host)
 
 #define MAX_CMD_LEN 1024
 
-char *get_port(char *host)
+static char *get_port(char *host)
 {
 	char *end;
 	char *p = strchr(host, ':');
diff --git a/daemon.c b/daemon.c
index b2babcc..366db37 100644
--- a/daemon.c
+++ b/daemon.c
@@ -453,7 +453,7 @@ static void parse_host_arg(char *extra_args, int buflen)
 		memset(&hints, 0, sizeof(hints));
 		hints.ai_flags = AI_CANONNAME;
 
-		gai = getaddrinfo(hostname, 0, &hints, &ai);
+		gai = getaddrinfo(hostname, NULL, &hints, &ai);
 		if (!gai) {
 			struct sockaddr_in *sin_addr = (void *)ai->ai_addr;
 
diff --git a/imap-send.c b/imap-send.c
index e4c83b9..3847fd1 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -238,7 +238,7 @@ static const char *Flags[] = {
 #ifndef NO_OPENSSL
 static void ssl_socket_perror(const char *func)
 {
-	fprintf(stderr, "%s: %s\n", func, ERR_error_string(ERR_get_error(), 0));
+	fprintf(stderr, "%s: %s\n", func, ERR_error_string(ERR_get_error(), NULL));
 }
 #endif
 
diff --git a/index-pack.c b/index-pack.c
index 6e93ee6..4d85aeb 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -469,7 +469,7 @@ static void sha1_object(const void *data, unsigned long size,
 				die("invalid %s", typename(type));
 			if (fsck_object(obj, 1, fsck_error_function))
 				die("Error in object");
-			if (fsck_walk(obj, mark_link, 0))
+			if (fsck_walk(obj, mark_link, NULL))
 				die("Not all child objects of %s are reachable", sha1_to_hex(obj->sha1));
 
 			if (obj->type == OBJ_TREE) {
diff --git a/mailmap.c b/mailmap.c
index bb1f2fb..f167c00 100644
--- a/mailmap.c
+++ b/mailmap.c
@@ -103,7 +103,7 @@ static char *parse_name_and_email(char *buffer, char **name,
 		char **email, int allow_empty_email)
 {
 	char *left, *right, *nstart, *nend;
-	*name = *email = 0;
+	*name = *email = NULL;
 
 	if ((left = strchr(buffer, '<')) == NULL)
 		return NULL;
@@ -136,7 +136,7 @@ static int read_single_mailmap(struct string_list *map, const char *filename, ch
 	if (f == NULL)
 		return 1;
 	while (fgets(buffer, sizeof(buffer), f) != NULL) {
-		char *name1 = 0, *email1 = 0, *name2 = 0, *email2 = 0;
+		char *name1 = NULL, *email1 = NULL, *name2 = NULL, *email2 = NULL;
 		if (buffer[0] == '#') {
 			static const char abbrev[] = "# repo-abbrev:";
 			int abblen = sizeof(abbrev) - 1;
@@ -200,7 +200,7 @@ int map_user(struct string_list *map,
 	if (!p) {
 		/* email passed in might not be wrapped in <>, but end with a \0 */
 		p = memchr(email, '\0', maxlen_email);
-		if (p == 0)
+		if (!p)
 			return 0;
 	}
 	if (p - email + 1 < sizeof(buf))
diff --git a/merge-recursive.c b/merge-recursive.c
index f5df9b9..c703445 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -38,7 +38,7 @@ static struct tree *shift_tree_object(struct tree *one, struct tree *two)
  * A virtual commit has (const char *)commit->util set to the name.
  */
 
-struct commit *make_virtual_commit(struct tree *tree, const char *comment)
+static struct commit *make_virtual_commit(struct tree *tree, const char *comment)
 {
 	struct commit *commit = xcalloc(1, sizeof(struct commit));
 	commit->tree = tree;
diff --git a/parse-options.c b/parse-options.c
index 79de18b..4eefdb1 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -478,7 +478,7 @@ static int usage_argh(const struct option *opts)
 #define USAGE_OPTS_WIDTH 24
 #define USAGE_GAP         2
 
-int usage_with_options_internal(const char * const *usagestr,
+static int usage_with_options_internal(const char * const *usagestr,
 				const struct option *opts, int full)
 {
 	if (!usagestr)
diff --git a/quote.c b/quote.c
index 7a49fcf..48bce2e 100644
--- a/quote.c
+++ b/quote.c
@@ -272,8 +272,8 @@ void write_name_quoted(const char *name, FILE *fp, int terminator)
 	fputc(terminator, fp);
 }
 
-extern void write_name_quotedpfx(const char *pfx, size_t pfxlen,
-                                 const char *name, FILE *fp, int terminator)
+void write_name_quotedpfx(const char *pfx, size_t pfxlen,
+                          const char *name, FILE *fp, int terminator)
 {
 	int needquote = 0;
 
diff --git a/remote.c b/remote.c
index 08a5964..c2a2846 100644
--- a/remote.c
+++ b/remote.c
@@ -301,7 +301,7 @@ static void read_branches_file(struct remote *remote)
 		strbuf_addstr(&branch, "HEAD:");
 	}
 	add_url_alias(remote, p);
-	add_fetch_refspec(remote, strbuf_detach(&branch, 0));
+	add_fetch_refspec(remote, strbuf_detach(&branch, NULL));
 	/*
 	 * Cogito compatible push: push current HEAD to remote #branch
 	 * (master if missing)
@@ -312,7 +312,7 @@ static void read_branches_file(struct remote *remote)
 		strbuf_addf(&branch, ":refs/heads/%s", frag);
 	else
 		strbuf_addstr(&branch, ":refs/heads/master");
-	add_push_refspec(remote, strbuf_detach(&branch, 0));
+	add_push_refspec(remote, strbuf_detach(&branch, NULL));
 	remote->fetch_tags = 1; /* always auto-follow */
 }
 
@@ -1105,7 +1105,7 @@ int match_refs(struct ref *src, struct ref **dst,
 	int send_all = flags & MATCH_REFS_ALL;
 	int send_mirror = flags & MATCH_REFS_MIRROR;
 	int errs;
-	static const char *default_refspec[] = { ":", 0 };
+	static const char *default_refspec[] = { ":", NULL };
 	struct ref **dst_tail = tail_ref(dst);
 
 	if (!nr_refspec) {
diff --git a/test-parse-options.c b/test-parse-options.c
index a90bc30..efa734b 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -9,7 +9,7 @@ static int verbose = 0, dry_run = 0, quiet = 0;
 static char *string = NULL;
 static char *file = NULL;
 
-int length_callback(const struct option *opt, const char *arg, int unset)
+static int length_callback(const struct option *opt, const char *arg, int unset)
 {
 	printf("Callback: \"%s\", %d\n",
 		(arg ? arg : "not set"), unset);
@@ -20,7 +20,7 @@ int length_callback(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
-int number_callback(const struct option *opt, const char *arg, int unset)
+static int number_callback(const struct option *opt, const char *arg, int unset)
 {
 	*(int *)opt->value = strtol(arg, NULL, 10);
 	return 0;

Re: Fix various sparse warnings in the git source code

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 22:46:58 


On Thu, 18 Jun 2009, Linus Torvalds wrote:
 - warning: symbol 'xyz' was not declared. Should it be static?

   Sparse wants to see declarations for any functions you export. A lack 
   of a declaration tends to mean that you should either add one, or you 
   should mark the function 'static' to show that it's in file scope. 

   A few of these remain: I only did the ones that should obviously just 
   be made static.
I don't know why I missed a couple.

That 'wt_status_submodule_summary' one is debatable. It has a few related 
flags (like 'wt_status_use_color') which _are_ declared, and are used by 
builtin-commit.c. So maybe we'd like to export it at some point, but it's 
not declared now, and not used outside of that file, so 'static' it is in 
this patch.

		Linus

---
 http-push.c    |    2 +-
 unpack-trees.c |    2 +-
 wt-status.c    |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/http-push.c b/http-push.c
index e4ea395..8cc8ee0 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1750,7 +1750,7 @@ static int delete_remote_branch(char *pattern, int force)
 	return 0;
 }
 
-void run_request_queue(void)
+static void run_request_queue(void)
 {
 #ifdef USE_CURL_MULTI
 	is_running_queue = 1;
diff --git a/unpack-trees.c b/unpack-trees.c
index ac9927f..1958319 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -140,7 +140,7 @@ static int unpack_index_entry(struct cache_entry *ce, struct unpack_trees_option
 	return call_unpack_fn(src, o);
 }
 
-int traverse_trees_recursive(int n, unsigned long dirmask, unsigned long df_conflicts, struct name_entry *names, struct traverse_info *info)
+static int traverse_trees_recursive(int n, unsigned long dirmask, unsigned long df_conflicts, struct name_entry *names, struct traverse_info *info)
 {
 	int i;
 	struct tree_desc t[MAX_UNPACK_TREES];
diff --git a/wt-status.c b/wt-status.c
index 24a6abf..47735d8 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -13,7 +13,7 @@
 
 int wt_status_relative_paths = 1;
 int wt_status_use_color = -1;
-int wt_status_submodule_summary;
+static int wt_status_submodule_summary;
 static char wt_status_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_NORMAL, /* WT_STATUS_HEADER */
 	GIT_COLOR_GREEN,  /* WT_STATUS_UPDATED */
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help