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

[Vladimir 'phcoder' Serbinenko]

Le 9 nov. 2015 8:39 AM, "Paulo Flabiano Smorigo" <address@hidden> a écrit :
>
> All SAS disks attached will be added to the device list. This is the second
> version of a patch that I send a while ago [1].
>
> [1] https://lists.gnu.org/archive/html/grub-devel/2015-08/msg00000.html
>
> ---
>  grub-core/disk/ieee1275/ofdisk.c | 76 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 76 insertions(+)
>
> diff --git a/grub-core/disk/ieee1275/ofdisk.c b/grub-core/disk/ieee1275/ofdisk.c
> index 297f058..afec9ba 100644
> --- a/grub-core/disk/ieee1275/ofdisk.c
> +++ b/grub-core/disk/ieee1275/ofdisk.c
> @@ -260,6 +260,82 @@ 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_uint32_t table_size;
> +      grub_ieee1275_ihandle_t ihandle;
> +
> +      buf = grub_malloc (grub_strlen (alias->path)
> +                         + sizeof ("/address@hidden"));
> +      if (!buf)
> +        return;
> +
> +      /* 64 entries should be enough */
Why? Sounds like an arbitrary limit. What does OF call return when it's not true? Does it set nentries to total or to how much it has really provided?
> +      table_size = sizeof (grub_uint64_t) * 64;
> +      table = grub_malloc (table_size);
> +
> +      if (!table)
> +        {
> +          grub_free (buf);
> +          return;
> +        }
> +
> +      bufptr = grub_stpcpy (buf, alias->path);
> +
> +      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;
Please double check that this field is in bytes.
> +      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);
This 32 doesn't match the above allocation.
> +          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)
> --
> 2.1.0
>
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel