Re: [PATCH,HURD] Fix grub-probe with userland partition support

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 25.04.2012 11:37, Samuel Thibault wrote:
> +  /* Make sure the string is terminated.  */
> +  argz[argz_len-1] = 0;
This would fail if argz_len == 0. You have to check this case.
> +
> +  /* Skip first word (translator path) and options.  */
> +  for (i = strlen (&argz[0]) + 1; i < argz_len; i += strlen (&argz[i]) + 1)
> +    {
please use argz and argz + i
> +      if (argz[i] != '-')
> +        {
> +          /* Non-option.  Only accept one, assumed to be the FS path.  */
> +          /* XXX: this should be replaced by an RPC to the translator.  */
> +          if (name)
> +            /* TRANSLATORS: we expect to get something like
> +               /hurd/foobar --option1 --option2=baz /dev/something
> +             */
> +            grub_util_error (_("Translator `%s' for path `%s' has several "
> +                               "non-option words, at least `%s' and `%s'"),
> +                               &argz[0], path, name, &argz[i]);
> +          name = &argz[i];
ditto
> +        }
> +    }
> +
> +  if (!name)
> +    /* TRANSLATORS: we expect to get something like
> +       /hurd/foobar --option1 --option2=baz /dev/something
> +     */
> +    grub_util_error (_("Translator `%s' for path `%s' is given only options, 
> "
> +                       "cannot find device part"), &argz[0], path);
> +
> +  if (!strncmp (name, "device:", strlen ("device:")))
Please use explicit == 0 and use sizeof ("...")-1 instead of strlen on
const string.
> +    {
> +      char *dev_name = name + strlen ("device:");
> +      size_t size = strlen ("/dev/") + strlen (dev_name) + 1;
> +      ret = malloc (size);
> +      snprintf (ret, size, "/dev/%s", dev_name);
stpcpy is more appropriate than snprintf here.

> +    }
> +  else if (!strncmp (name, "file:", strlen ("file:")))
> +    ret = strdup (name + strlen ("file:"));
> +  else
> +    ret = strdup (name);
> +
> +  munmap (argz, argz_len);
> +  return ret;


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

signature.asc
Description: OpenPGP digital signature