Re: [PATCH] Extend SystemTap SDT probe argument parser

[Pedro Alves <palves@redhat.com>]

On 12/17/2013 05:27 PM, Sergio Durigan Junior wrote:
> +/* Helper function to print a list of strings, represented as "const
> +   char *const *".  The list is printed comma-separated.  */
> +
> +static char *
> +pstring_list (const char *const *list)
> +{
> +  static char ret[100];
> +  const char *const *p;
> +  size_t offset = 0;
> +
> +  if (list == NULL)
> +    return "(null)";
> +
> +  ret[0] = '\0';
> +  for (p = list; *p != NULL && offset < sizeof (ret); ++p)
> +    {
> +      size_t s = xsnprintf (ret + offset, sizeof (ret) - offset, "%s, ", *p);
> +      offset += 2 + s;
> +    }
> +
> +  gdb_assert (offset - 2 < sizeof (ret));

Note this will assert if the list is empty (but not NULL), i.e., { NULL },
because offset will be 0, and "offset - 2" will wrap around
(offset is unsigned size_t.)  I suggest either moving the assert within
the if below, or handle that case especially, printing "(empty)"
or some such.

> +  if (offset > 0)
> +    ret[offset - 2] = '\0';
> +
> +  return ret;

I have no further comments.

Thanks,
-- 
Pedro Alves