Re: [PATCH] Don't allow fast-import tree delta chains to exceed maximum depth
Junio C Hamano <gitster@pobox.com> wrote:
Show 6 quoted lines
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> > + uint32_t type : TYPE_BITS,
> > + pack_id : PACK_ID_BITS,
> > + depth : DEPTH_BITS;
>
> uint32_t with bit-width specifiers look somewhat funny here...
Sure. But the prior item is also uint32_t and before that a pointer
so it aligns out nice. And we're using exactly 32 bits in this field.
So it just sort of made sense to me to declare it was a uint32_t.
Show 10 quoted lines
> > @@ -1105,7 +1108,7 @@ static int store_object(
> > unsigned pos = sizeof(hdr) - 1;
> >
> > delta_count_by_type[type]++;
> > - last->depth++;
> > + e->depth = ++last->depth++;
>
> "lvalue required as increment operand"?
>
> Wouldn't it be easier to read like this?
Show 39 quoted lines
> diff --git a/fast-import.c b/fast-import.c
> index 7544949..d32c412 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -161,7 +161,7 @@ struct object_entry
> {
> struct object_entry *next;
> uint32_t offset;
> - uint32_t type : TYPE_BITS,
> + unsigned type : TYPE_BITS,
> pack_id : PACK_ID_BITS,
> depth : DEPTH_BITS;
> unsigned char sha1[20];
> @@ -1108,7 +1108,7 @@ static int store_object(
> unsigned pos = sizeof(hdr) - 1;
>
> delta_count_by_type[type]++;
> - e->depth = ++last->depth++;
> + e->depth = last->depth + 1;
>
> hdrlen = encode_header(OBJ_OFS_DELTA, deltalen, hdr);
> write_or_die(pack_data->pack_fd, hdr, hdrlen);
> @@ -1121,8 +1121,6 @@ static int store_object(
> pack_size += sizeof(hdr) - pos;
> } else {
> e->depth = 0;
> - if (last)
> - last->depth = 0;
> hdrlen = encode_header(type, datlen, hdr);
> write_or_die(pack_data->pack_fd, hdr, hdrlen);
> pack_size += hdrlen;
> @@ -1138,6 +1136,7 @@ static int store_object(
> free(last->data);
> last->data = dat;
> last->offset = e->offset;
> + last->depth = e->depth;
> last->len = datlen;
> }
> return 0;--
Shawn.
Shawn O. Pearce <spearce@spearce.org>