Web lists-archives.com

Re: [PATCH 3/9] Convert unpack-objects to size_t




On 12 August 2017 at 10:47, Martin Koegler <martin.koegler@xxxxxxxxx> wrote:
> From: Martin Koegler <martin.koegler@xxxxxxxxx>
>
> Signed-off-by: Martin Koegler <martin.koegler@xxxxxxxxx>
> ---
>  builtin/unpack-objects.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
> index 001dd4b..0d8b6b3 100644
> --- a/builtin/unpack-objects.c
> +++ b/builtin/unpack-objects.c
> @@ -31,7 +31,7 @@ static struct fsck_options fsck_options = FSCK_OPTIONS_STRICT;
>   */
>  struct obj_buffer {
>         char *buffer;
> -       unsigned long size;
> +       size_t size;
>  };
>
>  static struct decoration obj_decorate;
> @@ -41,7 +41,7 @@ static struct obj_buffer *lookup_object_buffer(struct object *base)
>         return lookup_decoration(&obj_decorate, base);
>  }
>
> -static void add_object_buffer(struct object *object, char *buffer, unsigned long size)
> +static void add_object_buffer(struct object *object, char *buffer, size_t size)
>  {
>         struct obj_buffer *obj;
>         obj = xcalloc(1, sizeof(struct obj_buffer));
> @@ -93,7 +93,7 @@ static void use(int bytes)
>                 die(_("pack exceeds maximum allowed size"));
>  }
>
> -static void *get_data(unsigned long size)
> +static void *get_data(size_t size)
>  {
>         git_zstream stream;
>         void *buf = xmallocz(size);

"size" is handed over to a "git_zstream" and goes through zlib.c,
eventually ending up in zlib, which is outside Git's control, and which
seems to work with "uLong"s. How do these kind of changes interact with
zlib? For example, I wonder about this line further down in get_data:

if (stream.total_out == size && ret == Z_STREAM_END)

If total_out isn't converted, I guess this would never hit if "size" is
too large. And if total_out /is/ converted, I guess we'd risk truncation
in zlib_pre_call in zlib.c. Maybe that might cause Git and zlib to have
different ideas about how much data is available and/or should be
processed. Maybe we could then hit things like this in git.c:

if (s->z.total_out != s->total_out + bytes_produced)
        die("BUG: total_out mismatch");

I am not very familiar with zlib, so apologies if this is just noise...

Martin