BTW, I dropped the Debian bugbot from CC.

Mike  Dupont <[email protected]> writes:

> @@ -207,8 +202,8 @@ param_printf_next(struct param_enum *sel
>              continue;
>
>          case '*':
> -            /* Length parameter given in the next
> -             * argument.  */
> +            /* Min or max Length parameter given in the next
> +             * argument depending on position.  */

I don't understand the intention behind this commentary change.
(Also, s/Length/length/.)

>              if (self->future_length == NULL)
>                  /* This should really be an assert,
>                   * but we can't just fail on invalid
> @@ -229,8 +224,6 @@ param_printf_next(struct param_enum *sel
>              /* Field length likewise, but we need to parse
>               * this to attach the appropriate string
>               * length expression.  */
> -            if (len_buf_len < sizeof(len_buf) - 1)
> -                len_buf[len_buf_len++] = *self->ptr;

The comment is not accurate anymore.  I think you can move all the
numeric cases up in the switch alongside the '#', ' ' etc.

Looks good otherwise.

Thanks,
Petr

_______________________________________________
Ltrace-devel mailing list
[email protected]
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/ltrace-devel

Reply via email to