* [PATCH 1/3] [Enhancement] Improve internals / refactoring.
@ 2019-02-01 8:36 Shahzad Lone
2019-02-01 8:36 ` [PATCH 3/3] It's C not C++ so variable length array should not be used [-Werror=vla] :,) Shahzad Lone
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Shahzad Lone @ 2019-02-01 8:36 UTC (permalink / raw)
To: git
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/3] It's C not C++ so variable length array should not be used [-Werror=vla] :,).
2019-02-01 8:36 [PATCH 1/3] [Enhancement] Improve internals / refactoring Shahzad Lone
@ 2019-02-01 8:36 ` Shahzad Lone
2019-02-01 9:02 ` Eric Sunshine
2019-02-01 8:36 ` [PATCH 2/3] ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] so fix that Shahzad Lone
2019-02-01 23:52 ` [PATCH v2 [rebased]] [Enhancement] Improve internals / refactoring Shahzad Lone
2 siblings, 1 reply; 9+ messages in thread
From: Shahzad Lone @ 2019-02-01 8:36 UTC (permalink / raw)
To: git
---
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] It's C not C++ so variable length array should not be used [-Werror=vla] :,).
2019-02-01 8:36 ` [PATCH 3/3] It's C not C++ so variable length array should not be used [-Werror=vla] :,) Shahzad Lone
@ 2019-02-01 9:02 ` Eric Sunshine
0 siblings, 0 replies; 9+ messages in thread
From: Eric Sunshine @ 2019-02-01 9:02 UTC (permalink / raw)
To: Shahzad Lone; +Cc: Git List
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] so fix that.
2019-02-01 8:36 [PATCH 1/3] [Enhancement] Improve internals / refactoring Shahzad Lone
2019-02-01 8:36 ` [PATCH 3/3] It's C not C++ so variable length array should not be used [-Werror=vla] :,) Shahzad Lone
@ 2019-02-01 8:36 ` Shahzad Lone
2019-02-01 9:00 ` Eric Sunshine
2019-02-01 23:52 ` [PATCH v2 [rebased]] [Enhancement] Improve internals / refactoring Shahzad Lone
2 siblings, 1 reply; 9+ messages in thread
From: Shahzad Lone @ 2019-02-01 8:36 UTC (permalink / raw)
To: git
---
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 2/3] ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] so fix that.
2019-02-01 8:36 ` [PATCH 2/3] ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] so fix that Shahzad Lone
@ 2019-02-01 9:00 ` Eric Sunshine
0 siblings, 0 replies; 9+ messages in thread
From: Eric Sunshine @ 2019-02-01 9:00 UTC (permalink / raw)
To: Shahzad Lone; +Cc: Git List
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).
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 [rebased]] [Enhancement] Improve internals / refactoring.
2019-02-01 8:36 [PATCH 1/3] [Enhancement] Improve internals / refactoring Shahzad Lone
2019-02-01 8:36 ` [PATCH 3/3] It's C not C++ so variable length array should not be used [-Werror=vla] :,) Shahzad Lone
2019-02-01 8:36 ` [PATCH 2/3] ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] so fix that Shahzad Lone
@ 2019-02-01 23:52 ` Shahzad Lone
2019-02-02 3:09 ` Brandon Richardson
2019-02-02 23:16 ` [PATCH v3 [re-fixed] ] " Shahzad Lone
2 siblings, 2 replies; 9+ messages in thread
From: Shahzad Lone @ 2019-02-01 23:52 UTC (permalink / raw)
To: git
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 [rebased]] [Enhancement] Improve internals / refactoring.
2019-02-01 23:52 ` [PATCH v2 [rebased]] [Enhancement] Improve internals / refactoring Shahzad Lone
@ 2019-02-02 3:09 ` Brandon Richardson
2019-02-02 23:16 ` [PATCH v3 [re-fixed] ] " Shahzad Lone
1 sibling, 0 replies; 9+ messages in thread
From: Brandon Richardson @ 2019-02-02 3:09 UTC (permalink / raw)
To: Shahzad Lone; +Cc: Git Mailing List
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 [re-fixed] ] [Enhancement] Improve internals / refactoring.
2019-02-01 23:52 ` [PATCH v2 [rebased]] [Enhancement] Improve internals / refactoring Shahzad Lone
2019-02-02 3:09 ` Brandon Richardson
@ 2019-02-02 23:16 ` Shahzad Lone
2019-02-04 17:56 ` Junio C Hamano
1 sibling, 1 reply; 9+ messages in thread
From: Shahzad Lone @ 2019-02-02 23:16 UTC (permalink / raw)
To: git
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v3 [re-fixed] ] [Enhancement] Improve internals / refactoring.
2019-02-02 23:16 ` [PATCH v3 [re-fixed] ] " Shahzad Lone
@ 2019-02-04 17:56 ` Junio C Hamano
0 siblings, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2019-02-04 17:56 UTC (permalink / raw)
To: Shahzad Lone; +Cc: git
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?
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-02-04 17:56 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-01 8:36 [PATCH 1/3] [Enhancement] Improve internals / refactoring Shahzad Lone
2019-02-01 8:36 ` [PATCH 3/3] It's C not C++ so variable length array should not be used [-Werror=vla] :,) Shahzad Lone
2019-02-01 9:02 ` Eric Sunshine
2019-02-01 8:36 ` [PATCH 2/3] ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] so fix that Shahzad Lone
2019-02-01 9:00 ` Eric Sunshine
2019-02-01 23:52 ` [PATCH v2 [rebased]] [Enhancement] Improve internals / refactoring Shahzad Lone
2019-02-02 3:09 ` Brandon Richardson
2019-02-02 23:16 ` [PATCH v3 [re-fixed] ] " Shahzad Lone
2019-02-04 17:56 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).