* [PATCH] multi-pack-index: make code -Wunused-parameter clean
@ 2018-11-04 0:49 Carlo Marcelo Arenas Belón
2018-11-04 2:27 ` Jeff King
0 siblings, 1 reply; 3+ messages in thread
From: Carlo Marcelo Arenas Belón @ 2018-11-04 0:49 UTC (permalink / raw)
To: git; +Cc: stolee, Carlo Marcelo Arenas Belón
introduced in 662148c435 ("midx: write object offsets", 2018-07-12)
but included on all previous versions as well.
midx.c:713:54: warning: unused parameter 'nr_objects' [-Wunused-parameter]
likely an oversight as the information needed to iterate over is
embedded in nr_large_offset
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
midx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/midx.c b/midx.c
index 4fac0cd08a..a2c17e3108 100644
--- a/midx.c
+++ b/midx.c
@@ -710,7 +710,7 @@ static size_t write_midx_object_offsets(struct hashfile *f, int large_offset_nee
}
static size_t write_midx_large_offsets(struct hashfile *f, uint32_t nr_large_offset,
- struct pack_midx_entry *objects, uint32_t nr_objects)
+ struct pack_midx_entry *objects)
{
struct pack_midx_entry *list = objects;
size_t written = 0;
@@ -880,7 +880,7 @@ int write_midx_file(const char *object_dir)
break;
case MIDX_CHUNKID_LARGEOFFSETS:
- written += write_midx_large_offsets(f, num_large_offsets, entries, nr_entries);
+ written += write_midx_large_offsets(f, num_large_offsets, entries);
break;
default:
--
2.19.1.816.gcd69ec8cd
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] multi-pack-index: make code -Wunused-parameter clean
2018-11-04 0:49 [PATCH] multi-pack-index: make code -Wunused-parameter clean Carlo Marcelo Arenas Belón
@ 2018-11-04 2:27 ` Jeff King
2018-11-05 16:48 ` Derrick Stolee
0 siblings, 1 reply; 3+ messages in thread
From: Jeff King @ 2018-11-04 2:27 UTC (permalink / raw)
To: Carlo Marcelo Arenas Belón; +Cc: git, stolee
On Sat, Nov 03, 2018 at 05:49:57PM -0700, Carlo Marcelo Arenas Belón wrote:
> introduced in 662148c435 ("midx: write object offsets", 2018-07-12)
> but included on all previous versions as well.
>
> midx.c:713:54: warning: unused parameter 'nr_objects' [-Wunused-parameter]
>
> likely an oversight as the information needed to iterate over is
> embedded in nr_large_offset
I've been preparing a series to make the whole code base compile with
-Wunused-parameter, and I handled this case a bit differently.
-- >8 --
Subject: [PATCH] midx: double-check large object write loop
The write_midx_large_offsets() function takes an array of object
entries, the number of entries in the array (nr_objects), and the number
of entries with large offsets (nr_large_offset). But we never actually
use nr_objects; instead we keep walking down the array and counting down
nr_large_offset until we've seen all of the large entries.
This is correct, but we can be a bit more defensive. If there were ever
a mismatch between nr_large_offset and the actual set of large-offset
objects, we'd walk off the end of the array.
Since we know the size of the array, we can use nr_objects to make sure
we don't walk too far.
Signed-off-by: Jeff King <peff@peff.net>
---
midx.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/midx.c b/midx.c
index 4fac0cd08a..ecd583666a 100644
--- a/midx.c
+++ b/midx.c
@@ -712,12 +712,18 @@ static size_t write_midx_object_offsets(struct hashfile *f, int large_offset_nee
static size_t write_midx_large_offsets(struct hashfile *f, uint32_t nr_large_offset,
struct pack_midx_entry *objects, uint32_t nr_objects)
{
- struct pack_midx_entry *list = objects;
+ struct pack_midx_entry *list = objects, *end = objects + nr_objects;
size_t written = 0;
while (nr_large_offset) {
- struct pack_midx_entry *obj = list++;
- uint64_t offset = obj->offset;
+ struct pack_midx_entry *obj;
+ uint64_t offset;
+
+ if (list >= end)
+ BUG("too many large-offset objects");
+
+ obj = list++;
+ offset = obj->offset;
if (!(offset >> 31))
continue;
--
2.19.1.1352.g60f3b1a4c2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] multi-pack-index: make code -Wunused-parameter clean
2018-11-04 2:27 ` Jeff King
@ 2018-11-05 16:48 ` Derrick Stolee
0 siblings, 0 replies; 3+ messages in thread
From: Derrick Stolee @ 2018-11-05 16:48 UTC (permalink / raw)
To: Jeff King, Carlo Marcelo Arenas Belón; +Cc: git
On 11/3/2018 10:27 PM, Jeff King wrote:
> On Sat, Nov 03, 2018 at 05:49:57PM -0700, Carlo Marcelo Arenas Belón wrote:
>
>> introduced in 662148c435 ("midx: write object offsets", 2018-07-12)
>> but included on all previous versions as well.
>>
>> midx.c:713:54: warning: unused parameter 'nr_objects' [-Wunused-parameter]
>>
>> likely an oversight as the information needed to iterate over is
>> embedded in nr_large_offset
> I've been preparing a series to make the whole code base compile with
> -Wunused-parameter, and I handled this case a bit differently.
>
> -- >8 --
> Subject: [PATCH] midx: double-check large object write loop
>
> The write_midx_large_offsets() function takes an array of object
> entries, the number of entries in the array (nr_objects), and the number
> of entries with large offsets (nr_large_offset). But we never actually
> use nr_objects; instead we keep walking down the array and counting down
> nr_large_offset until we've seen all of the large entries.
>
> This is correct, but we can be a bit more defensive. If there were ever
> a mismatch between nr_large_offset and the actual set of large-offset
> objects, we'd walk off the end of the array.
>
> Since we know the size of the array, we can use nr_objects to make sure
> we don't walk too far.
>
> Signed-off-by: Jeff King <peff@peff.net>
Thanks, both, for catching this. I prefer the approach that adds defenses.
Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
> ---
> midx.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/midx.c b/midx.c
> index 4fac0cd08a..ecd583666a 100644
> --- a/midx.c
> +++ b/midx.c
> @@ -712,12 +712,18 @@ static size_t write_midx_object_offsets(struct hashfile *f, int large_offset_nee
> static size_t write_midx_large_offsets(struct hashfile *f, uint32_t nr_large_offset,
> struct pack_midx_entry *objects, uint32_t nr_objects)
> {
> - struct pack_midx_entry *list = objects;
> + struct pack_midx_entry *list = objects, *end = objects + nr_objects;
> size_t written = 0;
>
> while (nr_large_offset) {
> - struct pack_midx_entry *obj = list++;
> - uint64_t offset = obj->offset;
> + struct pack_midx_entry *obj;
> + uint64_t offset;
> +
> + if (list >= end)
> + BUG("too many large-offset objects");
> +
> + obj = list++;
> + offset = obj->offset;
>
> if (!(offset >> 31))
> continue;
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-11-05 16:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-04 0:49 [PATCH] multi-pack-index: make code -Wunused-parameter clean Carlo Marcelo Arenas Belón
2018-11-04 2:27 ` Jeff King
2018-11-05 16:48 ` Derrick Stolee
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.