threads / patch / 51809

v2, 4 partsam: fail if no author line is given in --rebasing mode

Subject: [PATCH v2 1/4] am: fail if no author line is given in --rebasing mode

## tl;dr

5 messages between Sep 5, 2019 and Sep 6, 2019. Diffs are folded; open one to read it.

replies: 4people: 1as markdown or json

Stephan Beyer· Sep 5, 2019, 22:48 UTC · lore
This prevents a potential segmentation fault.
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
 builtin/am.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Show changes to builtin/am.c +2 −2
diff --git a/builtin/am.c b/builtin/am.c
index 1aea657a7f..71da34913c 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1272,7 +1272,8 @@ static void get_commit_info(struct am_state *state, struct commit *commit)
 	buffer = logmsg_reencode(commit, NULL, get_commit_output_encoding());

 	ident_line = find_commit_header(buffer, "author", &ident_len);
-
+	if (!ident_line)
+		die(_("no author line"));
 	if (split_ident_line(&id, ident_line, ident_len) < 0)
 		die(_("invalid ident line: %.*s"), (int)ident_len, ident_line);

--
2.23.0.43.g31ebfd7ae6.dirty
Stephan Beyer· Sep 5, 2019, 22:48 UTC · re: Stephan Beyer · lore

[PATCH v2 3/4] pack-objects: fix maybe-uninitialized warning for index_pos

gcc 9.2.1 with -flto shows a maybe-uninitialized warning for index_pos in builtin/pack-objects.c's add_object_entry(). Tracking it down, the variable should be initialized in pack_objects.c's packlist_find().

The return value of locate_object_entry_hash(), which becomes index_pos, is either (in case of found = 1) the position where the (already included) OID is, or (in case of found = 0), index_pos is the position where the (not yet included) OID will be after insertion (which takes place in packlist_alloc() if the hash table is still large enough).

However, packlist_find() does not invoke locate_object_entry_hash() if the index size is zero (which might be the case on the first run). This is the only case where index_pos is undefined; and it is irrelevant since the first run will increase the size of the hash table to 1024 and then the undefined value index_pos is ignored.

This patch sets index_pos to zero on the first run to silence the warning.
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
 pack-objects.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
Show changes to pack-objects.c +4 −2
diff --git a/pack-objects.c b/pack-objects.c
index 52560293b6..726147a75d 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -74,8 +74,11 @@ struct object_entry *packlist_find(struct packing_data *pdata,
 	uint32_t i;
 	int found;

-	if (!pdata->index_size)
+	if (!pdata->index_size) {
+		if (index_pos)
+			*index_pos = 0; /* silence uninitialized warning */
 		return NULL;
+	}

 	i = locate_object_entry_hash(pdata, oid, &found);

--
2.23.0.43.g31ebfd7ae6.dirty
Stephan Beyer· Sep 5, 2019, 22:48 UTC · re: Stephan Beyer · lore

[PATCH v2 2/4] test-read-cache: fix maybe-uninitialized warning for namelen

This is done by removing namelen at all. It is only used once and simply strlen(name), hence we use strlen(name) directly.

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
 t/helper/test-read-cache.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
Show changes to t/helper/test-read-cache.c +2 −4
diff --git a/t/helper/test-read-cache.c b/t/helper/test-read-cache.c
index 7e79b555de..244977a29b 100644
--- a/t/helper/test-read-cache.c
+++ b/t/helper/test-read-cache.c
@@ -4,11 +4,10 @@

 int cmd__read_cache(int argc, const char **argv)
 {
-	int i, cnt = 1, namelen;
+	int i, cnt = 1;
 	const char *name = NULL;

 	if (argc > 1 && skip_prefix(argv[1], "--print-and-refresh=", &name)) {
-		namelen = strlen(name);
 		argc--;
 		argv++;
 	}
@@ -24,7 +23,7 @@ int cmd__read_cache(int argc, const char **argv)

 			refresh_index(&the_index, REFRESH_QUIET,
 				      NULL, NULL, NULL);
-			pos = index_name_pos(&the_index, name, namelen);
+			pos = index_name_pos(&the_index, name, strlen(name));
 			if (pos < 0)
 				die("%s not in index", name);
 			printf("%s is%s up to date\n", name,
--
2.23.0.43.g31ebfd7ae6.dirty
Stephan Beyer· Sep 5, 2019, 22:48 UTC · re: Stephan Beyer · lore

[PATCH v2 4/4] Silence false-positive maybe-uninitialized warnings found by gcc 9 -flto

gcc 9.2.1 with -flto flag suspects some uninitialized variables which become initialized in every code path where they are used. These false positives are "fixed" by this patch in the most naïve way.

This allows to compile git with gcc 9, link-time optimization, and using the DEVELOPER=1 switch (which sets -Werror).

Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
 bulk-checkin.c | 2 ++
 fast-import.c  | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)
Show changes to 2 files +4 −2

bulk-checkin.c, fast-import.c

diff --git a/bulk-checkin.c b/bulk-checkin.c
index 39ee7d6107..87fa28c227 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -200,6 +200,8 @@ static int deflate_to_pack(struct bulk_checkin_state *state,
 	struct hashfile_checkpoint checkpoint;
 	struct pack_idx_entry *idx = NULL;

+	checkpoint.offset = 0;
+
 	seekback = lseek(fd, 0, SEEK_CUR);
 	if (seekback == (off_t) -1)
 		return error("cannot find the current offset");
diff --git a/fast-import.c b/fast-import.c
index b44d6a467e..58f73f9105 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -903,7 +903,8 @@ static int store_object(
 	struct object_entry *e;
 	unsigned char hdr[96];
 	struct object_id oid;
-	unsigned long hdrlen, deltalen;
+	unsigned long hdrlen;
+	unsigned long deltalen = 0;
 	git_hash_ctx c;
 	git_zstream s;

--
2.23.0.43.g31ebfd7ae6.dirty
Stephan Beyer· Sep 6, 2019, 12:39 UTC · re: Stephan Beyer · lore

Re: [PATCH v2 1/4] am: fail if no author line is given in --rebasing mode

Hi,

just to make it also clear in this thread: you can ignore this patch series in favor of the better patch series by Peff [1] that has found its way to the mailing list at the same time.

1. https://public-inbox.org/git/20190905224859.GA28660@sigill.intra.peff.net/

Stephan

← back to recent threads