# [PATCH 3/3] It's C not C++ so variable length array should not be used [-Werror=vla] :,).

9 messages from 2019-02-01 to 2019-02-04. Participants: Shahzad Lone, Eric Sunshine, Brandon Richardson, Junio C Hamano.
Thread: https://gitlist.dev/t/50376

## Shahzad Lone, 2019-02-01 08:36

Subject: [PATCH 3/3] It's C not C++ so variable length array should not be used [-Werror=vla] :,).
Message-ID: <01020168a8338127-d50bd8d1-c178-4f13-86b1-9a191f64ded0-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020168a8338127-d50bd8d1-c178-4f13-86b1-9a191f64ded0-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020168a8338064-b1c054c4-3e05-4825-b8dc-636d9c63dcfc-000000@eu-west-1.amazonses.com>

```
---
 builtin/pack-objects.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index c1ec9ef3232cb..3017beb8236fa 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -197,9 +197,8 @@ static unsigned long write_large_blob_data(struct git_istream *st, struct hashfi
 					   const struct object_id *oid)
 {
 	git_zstream stream;
-	const unsigned bufsize = 16384;
-	unsigned char ibuf[bufsize];
-	unsigned char obuf[bufsize];
+	unsigned char ibuf[16384];
+	unsigned char obuf[16384];
 	unsigned long olen = 0;
 
 	git_deflate_init(&stream, pack_compression_level);

--
https://github.com/git/git/pull/572

```

## Shahzad Lone, 2019-02-01 08:36

Subject: [PATCH 2/3] ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] so fix that.
Message-ID: <01020168a83380fb-ecad8400-915d-497c-841f-096541747cd6-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020168a83380fb-ecad8400-915d-497c-841f-096541747cd6-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020168a8338064-b1c054c4-3e05-4825-b8dc-636d9c63dcfc-000000@eu-west-1.amazonses.com>

```
---
 pack-revindex.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pack-revindex.c b/pack-revindex.c
index 202981b39c6b6..40651ec9fac2e 100644
--- a/pack-revindex.c
+++ b/pack-revindex.c
@@ -186,9 +186,9 @@ int find_revindex_position(struct packed_git *p, off_t ofs)
 
 struct revindex_entry *find_pack_revindex(struct packed_git *p, off_t ofs)
 {
-
+	int pos;
 	load_pack_revindex(p);
-	const int pos = find_revindex_position(p, ofs);
+	pos = find_revindex_position(p, ofs);
 
 	if (pos < 0)
 		return NULL;

--
https://github.com/git/git/pull/572

```

## Shahzad Lone, 2019-02-01 08:36

Subject: [PATCH 1/3] [Enhancement] Improve internals / refactoring.
Message-ID: <01020168a8338064-b1c054c4-3e05-4825-b8dc-636d9c63dcfc-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020168a8338064-b1c054c4-3e05-4825-b8dc-636d9c63dcfc-000000%40eu-west-1.amazonses.com

```
Changed to ˋˋˋconstsˋˋˋ and tried to save arithmetic cost where I could.

Sorry my coding OCD bothered me when I didn't see them being ˋˋˋconstsˋˋˋ.
---
 builtin/diff.c           |  2 +-
 builtin/pack-objects.c   | 19 ++++++++++---------
 builtin/pack-redundant.c |  4 ++--
 pack-revindex.c          | 13 ++++++-------
 4 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index f0393bba23a7d..84a362ff5625b 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -102,7 +102,7 @@ static int builtin_diff_blobs(struct rev_info *revs,
 			      int argc, const char **argv,
 			      struct object_array_entry **blob)
 {
-	unsigned mode = canon_mode(S_IFREG | 0644);
+	const unsigned mode = canon_mode(S_IFREG | 0644);
 
 	if (argc > 1)
 		usage(builtin_diff_usage);
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 0a70d046043ec..c1ec9ef3232cb 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -197,8 +197,9 @@ static unsigned long write_large_blob_data(struct git_istream *st, struct hashfi
 					   const struct object_id *oid)
 {
 	git_zstream stream;
-	unsigned char ibuf[1024 * 16];
-	unsigned char obuf[1024 * 16];
+	const unsigned bufsize = 16384;
+	unsigned char ibuf[bufsize];
+	unsigned char obuf[bufsize];
 	unsigned long olen = 0;
 
 	git_deflate_init(&stream, pack_compression_level);
@@ -1901,10 +1902,10 @@ static int type_size_sort(const void *_a, const void *_b)
 {
 	const struct object_entry *a = *(struct object_entry **)_a;
 	const struct object_entry *b = *(struct object_entry **)_b;
-	enum object_type a_type = oe_type(a);
-	enum object_type b_type = oe_type(b);
-	unsigned long a_size = SIZE(a);
-	unsigned long b_size = SIZE(b);
+	const enum object_type a_type = oe_type(a);
+	const enum object_type b_type = oe_type(b);
+	const unsigned long a_size = SIZE(a);
+	const unsigned long b_size = SIZE(b);
 
 	if (a_type > b_type)
 		return -1;
@@ -1919,7 +1920,7 @@ static int type_size_sort(const void *_a, const void *_b)
 	if (a->preferred_base < b->preferred_base)
 		return 1;
 	if (use_delta_islands) {
-		int island_cmp = island_delta_cmp(&a->idx.oid, &b->idx.oid);
+		const int island_cmp = island_delta_cmp(&a->idx.oid, &b->idx.oid);
 		if (island_cmp)
 			return island_cmp;
 	}
@@ -2171,7 +2172,7 @@ static unsigned int check_delta_limit(struct object_entry *me, unsigned int n)
 	struct object_entry *child = DELTA_CHILD(me);
 	unsigned int m = n;
 	while (child) {
-		unsigned int c = check_delta_limit(child, n + 1);
+		const unsigned int c = check_delta_limit(child, n + 1);
 		if (m < c)
 			m = c;
 		child = DELTA_SIBLING(child);
@@ -2226,7 +2227,7 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,
 		while (window_memory_limit &&
 		       mem_usage > window_memory_limit &&
 		       count > 1) {
-			uint32_t tail = (idx + window - count) % window;
+			const uint32_t tail = (idx + window - count) % window;
 			mem_usage -= free_unpacked(array + tail);
 			count--;
 		}
diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index cf9a9aabd4eb2..11bc51456631e 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -166,7 +166,7 @@ static inline struct llist_item * llist_sorted_remove(struct llist *list, const
 	l = (hint == NULL) ? list->front : hint;
 	prev = NULL;
 	while (l) {
-		int cmp = oidcmp(l->oid, oid);
+		const int cmp = oidcmp(l->oid, oid);
 		if (cmp > 0) /* not in list, since sorted */
 			return prev;
 		if (!cmp) { /* found */
@@ -264,7 +264,7 @@ static void cmp_two_packs(struct pack_list *p1, struct pack_list *p2)
 	while (p1_off < p1->pack->num_objects * p1_step &&
 	       p2_off < p2->pack->num_objects * p2_step)
 	{
-		int cmp = hashcmp(p1_base + p1_off, p2_base + p2_off);
+		const int cmp = hashcmp(p1_base + p1_off, p2_base + p2_off);
 		/* cmp ~ p1 - p2 */
 		if (cmp == 0) {
 			p1_hint = llist_sorted_remove(p1->unique_objects,
diff --git a/pack-revindex.c b/pack-revindex.c
index 3c58784a5f4de..202981b39c6b6 100644
--- a/pack-revindex.c
+++ b/pack-revindex.c
@@ -119,7 +119,7 @@ static void sort_revindex(struct revindex_entry *entries, unsigned n, off_t max)
  */
 static void create_pack_revindex(struct packed_git *p)
 {
-	unsigned num_ent = p->num_objects;
+	const unsigned num_ent = p->num_objects;
 	unsigned i;
 	const char *index = p->index_data;
 	const unsigned hashsz = the_hash_algo->rawsz;
@@ -132,7 +132,7 @@ static void create_pack_revindex(struct packed_git *p)
 			(uint32_t *)(index + 8 + p->num_objects * (hashsz + 4));
 		const uint32_t *off_64 = off_32 + p->num_objects;
 		for (i = 0; i < num_ent; i++) {
-			uint32_t off = ntohl(*off_32++);
+			const uint32_t off = ntohl(*off_32++);
 			if (!(off & 0x80000000)) {
 				p->revindex[i].offset = off;
 			} else {
@@ -143,7 +143,7 @@ static void create_pack_revindex(struct packed_git *p)
 		}
 	} else {
 		for (i = 0; i < num_ent; i++) {
-			uint32_t hl = *((uint32_t *)(index + (hashsz + 4) * i));
+			const uint32_t hl = *((uint32_t *)(index + (hashsz + 4) * i));
 			p->revindex[i].offset = ntohl(hl);
 			p->revindex[i].nr = i;
 		}
@@ -168,10 +168,10 @@ int find_revindex_position(struct packed_git *p, off_t ofs)
 {
 	int lo = 0;
 	int hi = p->num_objects + 1;
-	struct revindex_entry *revindex = p->revindex;
+	const struct revindex_entry *revindex = p->revindex;
 
 	do {
-		unsigned mi = lo + (hi - lo) / 2;
+		const unsigned mi = lo + (hi - lo) / 2;
 		if (revindex[mi].offset == ofs) {
 			return mi;
 		} else if (ofs < revindex[mi].offset)
@@ -186,10 +186,9 @@ int find_revindex_position(struct packed_git *p, off_t ofs)
 
 struct revindex_entry *find_pack_revindex(struct packed_git *p, off_t ofs)
 {
-	int pos;
 
 	load_pack_revindex(p);
-	pos = find_revindex_position(p, ofs);
+	const int pos = find_revindex_position(p, ofs);
 
 	if (pos < 0)
 		return NULL;

--
https://github.com/git/git/pull/572

```

## Eric Sunshine, 2019-02-01 09:00

Subject: Re: [PATCH 2/3] ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] so fix that.
Message-ID: <CAPig+cTEZVt13XBQ9z7aQp0PAZQbN5xRm+PC16iSNNr=G473YQ@mail.gmail.com>
URL: https://gitlist.dev/e/CAPig%2BcTEZVt13XBQ9z7aQp0PAZQbN5xRm%2BPC16iSNNr%3DG473YQ%40mail.gmail.com
In-Reply-To: <01020168a83380fb-ecad8400-915d-497c-841f-096541747cd6-000000@eu-west-1.amazonses.com>

```
On Fri, Feb 1, 2019 at 3:36 AM Shahzad Lone <shahzadlone@gmail.com> wrote:
> diff --git a/pack-revindex.c b/pack-revindex.c
> @@ -186,9 +186,9 @@ int find_revindex_position(struct packed_git *p, off_t ofs)
>  struct revindex_entry *find_pack_revindex(struct packed_git *p, off_t ofs)
>  {
> -
> +       int pos;
>         load_pack_revindex(p);
> -       const int pos = find_revindex_position(p, ofs);
> +       pos = find_revindex_position(p, ofs);

Thanks, however, this code was fine until your patch 1/3 changed it to
have a declaration after statement. Rather than creating a new patch
to fix an earlier mistake in the same patch series, on this project,
the proper way to resolve such a problem is use "git rebase -i" to
adjust patch 1/3 to not introduce the problem in the first place and
drop this patch, and then (re-)submit the patch series.

Also, your Signed-off-by: is missing (see Documentation/SubmittingPatches).

```

## Eric Sunshine, 2019-02-01 09:02

Subject: Re: [PATCH 3/3] It's C not C++ so variable length array should not be used [-Werror=vla] :,).
Message-ID: <CAPig+cTyVu7maTXBLaeCK7V_42+QrDPUs57a3F_tdwRDMYtcaA@mail.gmail.com>
URL: https://gitlist.dev/e/CAPig%2BcTyVu7maTXBLaeCK7V_42%2BQrDPUs57a3F_tdwRDMYtcaA%40mail.gmail.com
In-Reply-To: <01020168a8338127-d50bd8d1-c178-4f13-86b1-9a191f64ded0-000000@eu-west-1.amazonses.com>

```
On Fri, Feb 1, 2019 at 3:36 AM Shahzad Lone <shahzadlone@gmail.com> wrote:
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> @@ -197,9 +197,8 @@ static unsigned long write_large_blob_data(struct git_istream *st, struct hashfi
>                                            const struct object_id *oid)
>  {
>         git_zstream stream;
> -       const unsigned bufsize = 16384;
> -       unsigned char ibuf[bufsize];
> -       unsigned char obuf[bufsize];
> +       unsigned char ibuf[16384];
> +       unsigned char obuf[16384];

Reiterating my comment on patch 2/3, this code was fine until your
patch 1/3 changed it declare a variable length array. Rather than
creating a new patch to fix an earlier mistake in the same patch
series, use "git rebase -i" to adjust patch 1/3 to not introduce the
problem in the first place and drop this patch, and then (re-)submit
the patch series.

Thanks.

```

## Shahzad Lone, 2019-02-01 23:52

Subject: [PATCH v2 [rebased]] [Enhancement] Improve internals / refactoring.
Message-ID: <01020168ab79f642-10a06c5b-c3f7-441e-86f8-bff5e41ac834-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020168ab79f642-10a06c5b-c3f7-441e-86f8-bff5e41ac834-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020168a8338064-b1c054c4-3e05-4825-b8dc-636d9c63dcfc-000000@eu-west-1.amazonses.com>

```
Changed to ˋˋˋconstsˋˋˋ and tried to save arithmetic cost where I could.

Sorry my coding OCD bothered me when I didn't see them being ˋˋˋconstsˋˋˋ.

Signed-off-by: Shahzad Lone <shahzadlone@gmail.com>
---
 builtin/diff.c           |  2 +-
 builtin/pack-objects.c   | 18 +++++++++---------
 builtin/pack-redundant.c |  4 ++--
 pack-revindex.c          | 11 +++++------
 4 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index f0393bba23a7d..84a362ff5625b 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -102,7 +102,7 @@ static int builtin_diff_blobs(struct rev_info *revs,
 			      int argc, const char **argv,
 			      struct object_array_entry **blob)
 {
-	unsigned mode = canon_mode(S_IFREG | 0644);
+	const unsigned mode = canon_mode(S_IFREG | 0644);
 
 	if (argc > 1)
 		usage(builtin_diff_usage);
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 0a70d046043ec..3017beb8236fa 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -197,8 +197,8 @@ static unsigned long write_large_blob_data(struct git_istream *st, struct hashfi
 					   const struct object_id *oid)
 {
 	git_zstream stream;
-	unsigned char ibuf[1024 * 16];
-	unsigned char obuf[1024 * 16];
+	unsigned char ibuf[16384];
+	unsigned char obuf[16384];
 	unsigned long olen = 0;
 
 	git_deflate_init(&stream, pack_compression_level);
@@ -1901,10 +1901,10 @@ static int type_size_sort(const void *_a, const void *_b)
 {
 	const struct object_entry *a = *(struct object_entry **)_a;
 	const struct object_entry *b = *(struct object_entry **)_b;
-	enum object_type a_type = oe_type(a);
-	enum object_type b_type = oe_type(b);
-	unsigned long a_size = SIZE(a);
-	unsigned long b_size = SIZE(b);
+	const enum object_type a_type = oe_type(a);
+	const enum object_type b_type = oe_type(b);
+	const unsigned long a_size = SIZE(a);
+	const unsigned long b_size = SIZE(b);
 
 	if (a_type > b_type)
 		return -1;
@@ -1919,7 +1919,7 @@ static int type_size_sort(const void *_a, const void *_b)
 	if (a->preferred_base < b->preferred_base)
 		return 1;
 	if (use_delta_islands) {
-		int island_cmp = island_delta_cmp(&a->idx.oid, &b->idx.oid);
+		const int island_cmp = island_delta_cmp(&a->idx.oid, &b->idx.oid);
 		if (island_cmp)
 			return island_cmp;
 	}
@@ -2171,7 +2171,7 @@ static unsigned int check_delta_limit(struct object_entry *me, unsigned int n)
 	struct object_entry *child = DELTA_CHILD(me);
 	unsigned int m = n;
 	while (child) {
-		unsigned int c = check_delta_limit(child, n + 1);
+		const unsigned int c = check_delta_limit(child, n + 1);
 		if (m < c)
 			m = c;
 		child = DELTA_SIBLING(child);
@@ -2226,7 +2226,7 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,
 		while (window_memory_limit &&
 		       mem_usage > window_memory_limit &&
 		       count > 1) {
-			uint32_t tail = (idx + window - count) % window;
+			const uint32_t tail = (idx + window - count) % window;
 			mem_usage -= free_unpacked(array + tail);
 			count--;
 		}
diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index cf9a9aabd4eb2..11bc51456631e 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -166,7 +166,7 @@ static inline struct llist_item * llist_sorted_remove(struct llist *list, const
 	l = (hint == NULL) ? list->front : hint;
 	prev = NULL;
 	while (l) {
-		int cmp = oidcmp(l->oid, oid);
+		const int cmp = oidcmp(l->oid, oid);
 		if (cmp > 0) /* not in list, since sorted */
 			return prev;
 		if (!cmp) { /* found */
@@ -264,7 +264,7 @@ static void cmp_two_packs(struct pack_list *p1, struct pack_list *p2)
 	while (p1_off < p1->pack->num_objects * p1_step &&
 	       p2_off < p2->pack->num_objects * p2_step)
 	{
-		int cmp = hashcmp(p1_base + p1_off, p2_base + p2_off);
+		const int cmp = hashcmp(p1_base + p1_off, p2_base + p2_off);
 		/* cmp ~ p1 - p2 */
 		if (cmp == 0) {
 			p1_hint = llist_sorted_remove(p1->unique_objects,
diff --git a/pack-revindex.c b/pack-revindex.c
index 3c58784a5f4de..40651ec9fac2e 100644
--- a/pack-revindex.c
+++ b/pack-revindex.c
@@ -119,7 +119,7 @@ static void sort_revindex(struct revindex_entry *entries, unsigned n, off_t max)
  */
 static void create_pack_revindex(struct packed_git *p)
 {
-	unsigned num_ent = p->num_objects;
+	const unsigned num_ent = p->num_objects;
 	unsigned i;
 	const char *index = p->index_data;
 	const unsigned hashsz = the_hash_algo->rawsz;
@@ -132,7 +132,7 @@ static void create_pack_revindex(struct packed_git *p)
 			(uint32_t *)(index + 8 + p->num_objects * (hashsz + 4));
 		const uint32_t *off_64 = off_32 + p->num_objects;
 		for (i = 0; i < num_ent; i++) {
-			uint32_t off = ntohl(*off_32++);
+			const uint32_t off = ntohl(*off_32++);
 			if (!(off & 0x80000000)) {
 				p->revindex[i].offset = off;
 			} else {
@@ -143,7 +143,7 @@ static void create_pack_revindex(struct packed_git *p)
 		}
 	} else {
 		for (i = 0; i < num_ent; i++) {
-			uint32_t hl = *((uint32_t *)(index + (hashsz + 4) * i));
+			const uint32_t hl = *((uint32_t *)(index + (hashsz + 4) * i));
 			p->revindex[i].offset = ntohl(hl);
 			p->revindex[i].nr = i;
 		}
@@ -168,10 +168,10 @@ int find_revindex_position(struct packed_git *p, off_t ofs)
 {
 	int lo = 0;
 	int hi = p->num_objects + 1;
-	struct revindex_entry *revindex = p->revindex;
+	const struct revindex_entry *revindex = p->revindex;
 
 	do {
-		unsigned mi = lo + (hi - lo) / 2;
+		const unsigned mi = lo + (hi - lo) / 2;
 		if (revindex[mi].offset == ofs) {
 			return mi;
 		} else if (ofs < revindex[mi].offset)
@@ -187,7 +187,6 @@ int find_revindex_position(struct packed_git *p, off_t ofs)
 struct revindex_entry *find_pack_revindex(struct packed_git *p, off_t ofs)
 {
 	int pos;
-
 	load_pack_revindex(p);
 	pos = find_revindex_position(p, ofs);
 

--
https://github.com/git/git/pull/572

```

## Brandon Richardson, 2019-02-02 03:09

Subject: Re: [PATCH v2 [rebased]] [Enhancement] Improve internals / refactoring.
Message-ID: <CAETBDP7jw_z_Nz7X-+jne7sYrJ7G92wWsB3S_ipf3XjJwxYxfw@mail.gmail.com>
URL: https://gitlist.dev/e/CAETBDP7jw_z_Nz7X-%2Bjne7sYrJ7G92wWsB3S_ipf3XjJwxYxfw%40mail.gmail.com
In-Reply-To: <01020168ab79f642-10a06c5b-c3f7-441e-86f8-bff5e41ac834-000000@eu-west-1.amazonses.com>

```
Hi Shahzad,

On Fri, 1 Feb 2019 at 19:54, Shahzad Lone <shahzadlone@gmail.com> wrote:
>         git_zstream stream;
> -       unsigned char ibuf[1024 * 16];
> -       unsigned char obuf[1024 * 16];
> +       unsigned char ibuf[16384];
> +       unsigned char obuf[16384];
>         unsigned long olen = 0;

This change also brings very little value because most compilers will
evaluate constant expressions such as this at compile time. A quick
google query on "Constant Folding" will tell you all about this.

I love optimization as much as the next guy, but I also feel an argument can
be made for readability too. The former version is far easier to see that
the array will be 16KB in size, where 16384 is more difficult to visualize.

```

## Shahzad Lone, 2019-02-02 23:16

Subject: [PATCH v3 [re-fixed] ] [Enhancement] Improve internals / refactoring.
Message-ID: <01020168b080030b-eb0a0634-f2b9-42ed-a3be-76995e7127eb-000000@eu-west-1.amazonses.com>
URL: https://gitlist.dev/e/01020168b080030b-eb0a0634-f2b9-42ed-a3be-76995e7127eb-000000%40eu-west-1.amazonses.com
In-Reply-To: <01020168ab79f642-10a06c5b-c3f7-441e-86f8-bff5e41ac834-000000@eu-west-1.amazonses.com>

```
Changed to ˋˋˋconstsˋˋˋ and tried to save arithmetic cost where I could.

Sorry my coding OCD bothered me when I didn't see them being ˋˋˋconstsˋˋˋ.

Signed-off-by: Shahzad Lone <shahzadlone@gmail.com>
---
 builtin/diff.c           |  2 +-
 builtin/pack-objects.c   | 14 +++++++-------
 builtin/pack-redundant.c |  4 ++--
 pack-revindex.c          | 10 +++++-----
 4 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index f0393bba23a7d..84a362ff5625b 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -102,7 +102,7 @@ static int builtin_diff_blobs(struct rev_info *revs,
 			      int argc, const char **argv,
 			      struct object_array_entry **blob)
 {
-	unsigned mode = canon_mode(S_IFREG | 0644);
+	const unsigned mode = canon_mode(S_IFREG | 0644);
 
 	if (argc > 1)
 		usage(builtin_diff_usage);
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 0a70d046043ec..5c406ab4945f5 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1901,10 +1901,10 @@ static int type_size_sort(const void *_a, const void *_b)
 {
 	const struct object_entry *a = *(struct object_entry **)_a;
 	const struct object_entry *b = *(struct object_entry **)_b;
-	enum object_type a_type = oe_type(a);
-	enum object_type b_type = oe_type(b);
-	unsigned long a_size = SIZE(a);
-	unsigned long b_size = SIZE(b);
+	const enum object_type a_type = oe_type(a);
+	const enum object_type b_type = oe_type(b);
+	const unsigned long a_size = SIZE(a);
+	const unsigned long b_size = SIZE(b);
 
 	if (a_type > b_type)
 		return -1;
@@ -1919,7 +1919,7 @@ static int type_size_sort(const void *_a, const void *_b)
 	if (a->preferred_base < b->preferred_base)
 		return 1;
 	if (use_delta_islands) {
-		int island_cmp = island_delta_cmp(&a->idx.oid, &b->idx.oid);
+		const int island_cmp = island_delta_cmp(&a->idx.oid, &b->idx.oid);
 		if (island_cmp)
 			return island_cmp;
 	}
@@ -2171,7 +2171,7 @@ static unsigned int check_delta_limit(struct object_entry *me, unsigned int n)
 	struct object_entry *child = DELTA_CHILD(me);
 	unsigned int m = n;
 	while (child) {
-		unsigned int c = check_delta_limit(child, n + 1);
+		const unsigned int c = check_delta_limit(child, n + 1);
 		if (m < c)
 			m = c;
 		child = DELTA_SIBLING(child);
@@ -2226,7 +2226,7 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,
 		while (window_memory_limit &&
 		       mem_usage > window_memory_limit &&
 		       count > 1) {
-			uint32_t tail = (idx + window - count) % window;
+			const uint32_t tail = (idx + window - count) % window;
 			mem_usage -= free_unpacked(array + tail);
 			count--;
 		}
diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index cf9a9aabd4eb2..11bc51456631e 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -166,7 +166,7 @@ static inline struct llist_item * llist_sorted_remove(struct llist *list, const
 	l = (hint == NULL) ? list->front : hint;
 	prev = NULL;
 	while (l) {
-		int cmp = oidcmp(l->oid, oid);
+		const int cmp = oidcmp(l->oid, oid);
 		if (cmp > 0) /* not in list, since sorted */
 			return prev;
 		if (!cmp) { /* found */
@@ -264,7 +264,7 @@ static void cmp_two_packs(struct pack_list *p1, struct pack_list *p2)
 	while (p1_off < p1->pack->num_objects * p1_step &&
 	       p2_off < p2->pack->num_objects * p2_step)
 	{
-		int cmp = hashcmp(p1_base + p1_off, p2_base + p2_off);
+		const int cmp = hashcmp(p1_base + p1_off, p2_base + p2_off);
 		/* cmp ~ p1 - p2 */
 		if (cmp == 0) {
 			p1_hint = llist_sorted_remove(p1->unique_objects,
diff --git a/pack-revindex.c b/pack-revindex.c
index 3c58784a5f4de..50891f77a26d6 100644
--- a/pack-revindex.c
+++ b/pack-revindex.c
@@ -119,7 +119,7 @@ static void sort_revindex(struct revindex_entry *entries, unsigned n, off_t max)
  */
 static void create_pack_revindex(struct packed_git *p)
 {
-	unsigned num_ent = p->num_objects;
+	const unsigned num_ent = p->num_objects;
 	unsigned i;
 	const char *index = p->index_data;
 	const unsigned hashsz = the_hash_algo->rawsz;
@@ -132,7 +132,7 @@ static void create_pack_revindex(struct packed_git *p)
 			(uint32_t *)(index + 8 + p->num_objects * (hashsz + 4));
 		const uint32_t *off_64 = off_32 + p->num_objects;
 		for (i = 0; i < num_ent; i++) {
-			uint32_t off = ntohl(*off_32++);
+			const uint32_t off = ntohl(*off_32++);
 			if (!(off & 0x80000000)) {
 				p->revindex[i].offset = off;
 			} else {
@@ -143,7 +143,7 @@ static void create_pack_revindex(struct packed_git *p)
 		}
 	} else {
 		for (i = 0; i < num_ent; i++) {
-			uint32_t hl = *((uint32_t *)(index + (hashsz + 4) * i));
+			const uint32_t hl = *((uint32_t *)(index + (hashsz + 4) * i));
 			p->revindex[i].offset = ntohl(hl);
 			p->revindex[i].nr = i;
 		}
@@ -168,10 +168,10 @@ int find_revindex_position(struct packed_git *p, off_t ofs)
 {
 	int lo = 0;
 	int hi = p->num_objects + 1;
-	struct revindex_entry *revindex = p->revindex;
+	const struct revindex_entry *revindex = p->revindex;
 
 	do {
-		unsigned mi = lo + (hi - lo) / 2;
+		const unsigned mi = lo + (hi - lo) / 2;
 		if (revindex[mi].offset == ofs) {
 			return mi;
 		} else if (ofs < revindex[mi].offset)

--
https://github.com/git/git/pull/572

```

## Junio C Hamano, 2019-02-04 17:56

Subject: Re: [PATCH v3 [re-fixed] ] [Enhancement] Improve internals / refactoring.
Message-ID: <xmqqbm3rwi7t.fsf@gitster-ct.c.googlers.com>
URL: https://gitlist.dev/e/xmqqbm3rwi7t.fsf%40gitster-ct.c.googlers.com
In-Reply-To: <01020168b080030b-eb0a0634-f2b9-42ed-a3be-76995e7127eb-000000@eu-west-1.amazonses.com>

```
Shahzad Lone <shahzadlone@gmail.com> writes:

> Subject: Re: [PATCH v3 [re-fixed] ] [Enhancement] Improve internals / refactoring.

Since all patches are attempts by their authors to improve
something, the above has a very low information density.  As all the
changes in this patch are about const-ness, it would probably be a
good idea to sneak that word into the title---that way, when placed
among many changes, a reader would be able to still recognize what
this change is about in "git shortlog --no-merges" output.

Perhaps

        Subject: [PATCH v3] tighten constness

or something?

```