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

["Vladimir 'φ-coder/phcoder' Serbinenko" <phcoder@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1657 bytes --]

On 24.04.2012 01:34, Samuel Thibault wrote:
> +/* Retrieve the storage information for PATH, and search which partitions
> + * contains its start.  */
> +static char *
> +find_hurd_root_device (const char *path)
> +{
> +  struct store *path_store, *part_store;
> +  char *devpath = NULL;
> +  const char *prefix;
> +  size_t len, prefix_len;
> +  store_offset_t path_start;
> +  int part;
> +
> +  path_store = storeinfo_store (path);
> +  if (! path_store)
> +    grub_util_error ("Could not retrieve storage information for `%s'", path);
> +  if (! path_store->name)
> +    grub_util_error ("No storage name associated with `%s'", path);
grub_util_error messages should be marked for translation. And probably
have a TRANSLATORS comment about the context.
> +
> +  grub_util_info ("Storage is %s, type %d",
> +      path_store->name, path_store->class->id);
> +
> +  if (path_store->class->id == STORAGE_DEVICE)
> +    prefix = "/dev/";
> +  else
> +    prefix = "";
> +
> +  prefix_len = strlen (prefix);
> +  len = strlen (path_store->name);
> +  devpath = xmalloc (prefix_len + len + 4 + 1);
It's better to have an explicit sizeof ("s999") rather than 4 + 1
> +	  if (errno == ENOENT && part >= 5)
> +	    {
Said in previous mail
> +      if (first_run->start * part_store->block_size <= path_start &&
> +	  (last_run->start + last_run->length) * part_store->block_size > path_start) {
Why this weird check rather than just check that start is the same?
Some partitions may contain more partitions, like logical container
contains logical partitions.

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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]