Re: [PATCH v4] ofdisk: add sas disks to the device list

[Andrei Borzenkov]

11.11.2015 16:14, Paulo Flabiano Smorigo пишет:
> Same patch with the fix.
> ---
>   grub-core/disk/ieee1275/ofdisk.c | 75 ++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 75 insertions(+)
>
> diff --git a/grub-core/disk/ieee1275/ofdisk.c b/grub-core/disk/ieee1275/ofdisk.c
> index 297f058..ef290bc 100644
> --- a/grub-core/disk/ieee1275/ofdisk.c
> +++ b/grub-core/disk/ieee1275/ofdisk.c
> @@ -260,6 +260,81 @@ dev_iterate (const struct grub_ieee1275_devalias *alias)
>         grub_free (buf);
>         return;
>       }
> +  else if (grub_strcmp (alias->type, "sas_ioa") == 0)
> +    {
> +      /* The method returns the number of disks and a table where
> +       * each ID is 64-bit long. Example of sas paths:
> +       *  /address@hidden/pci1014,address@hidden/sas/address@hidden
> +       *  /address@hidden/pci1014,address@hidden/sas/address@hidden
> +       *  /address@hidden/pci1014,address@hidden/sas/address@hidden */
> +
> +      struct sas_children
> +        {
> +          struct grub_ieee1275_common_hdr common;
> +          grub_ieee1275_cell_t method;
> +          grub_ieee1275_cell_t ihandle;
> +          grub_ieee1275_cell_t max;
> +          grub_ieee1275_cell_t table;
> +          grub_ieee1275_cell_t catch_result;
> +          grub_ieee1275_cell_t nentries;
> +        }
> +      args;
> +      char *buf, *bufptr, *table;
> +      unsigned i;
> +      grub_uint16_t table_size;
> +      grub_ieee1275_ihandle_t ihandle;
> +
> +      buf = grub_malloc (grub_strlen (alias->path) + 32);

Actually I liked previous version more :) Magic hidden constants are 
always error prone.

> +      if (!buf)
> +        return;
> +      bufptr = grub_stpcpy (buf, alias->path);
> +
> +      /* Power machines documentation specify 672 as maximum SAS disks in
> +         one system. Using a slightly larger value to be safe. */
> +      table_size = 768;
> +      table = grub_malloc (table_size * sizeof (grub_uint64_t));
> +
> +      if (!table)
> +        {
> +          grub_free (buf);
> +          return;
> +        }
> +
> +      if (grub_ieee1275_open (alias->path, &ihandle))
> +        {
> +          grub_free (buf);
> +          grub_free (table);
> +          return;
> +        }
> +
> +      INIT_IEEE1275_COMMON (&args.common, "call-method", 4, 2);
> +      args.method = (grub_ieee1275_cell_t) "get-sas-children";
> +      args.ihandle = ihandle;
> +      args.max = table_size;
> +      args.table = (grub_ieee1275_cell_t) table;
> +      args.catch_result = 0;
> +      args.nentries = 0;
> +
> +      if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
> +        {
> +          grub_ieee1275_close (ihandle);
> +          grub_free (table);
> +          grub_free (buf);
> +          return;
> +        }
> +
> +      grub_uint64_t *ptr;
> +      for (i = 0; i < args.nentries; i++)
> +        {
> +          ptr = (grub_uint64_t *) (table + sizeof (grub_uint64_t) * i);
> +          grub_snprintf (bufptr, 32, "/address@hidden" PRIxGRUB_UINT64_T, 
> *ptr);

What about

grub_snprintf (bufptr, sizeof ("/address@hidden"), ... )

> +          dev_iterate_real (buf, buf);
> +        }
> +
> +      grub_ieee1275_close (ihandle);
> +      grub_free (table);
> +      grub_free (buf);
> +    }
>
>     if (!grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_NO_TREE_SCANNING_FOR_DISKS)
>         && grub_strcmp (alias->type, "block") == 0)