[SeaBIOS] Re: [PATCH v7 6/8] bootdevice: Refactor get_boot_devices_list

[Philippe Mathieu-Daudé]

On 9/25/19 1:06 PM, Sam Eiderman wrote:
> From: Sam Eiderman
>
> Move device name construction to a separate function.
>
> We will reuse this function in the following commit to pass logical CHS
> parameters through fw_cfg much like we currently pass bootindex.
>
> Reviewed-by: Karl Heubaum 
> Reviewed-by: Arbel Moshe 
> Signed-off-by: Sam Eiderman
>
> bootdevice.c | 61 +++++++++++++++++++++++++++++-----------------------
>  1 file changed, 34 insertions(+), 27 deletions(-)
>
> diff --git a/bootdevice.c b/bootdevice.c
> index bc5e1c2de4..2b12fb85a4 100644
> --- a/bootdevice.c
> +++ b/bootdevice.c
> @@ -202,6 +202,39 @@ DeviceState *get_boot_device(uint32_t position)
>      return res;
>  }
>
> +static char *get_boot_device_path(DeviceState *dev, bool ignore_suffixes,
>                              char *suffix)

Please update to 'const char *suffix'.
John, can you do it directly in your tree before sending the pull request?
With it:
Reviewed-by: Philippe Mathieu-Daudé
> +{
> char *devpath = NULL, *s = NULL, *d, *bootpath;
> if (dev) {
>    devpath = qdev_get_fw_dev_path(dev);
>    assert(devpath);
> }
> if (!ignore_suffixes) {
>    if (dev) {
>        d = qdev_get_own_fw_dev_path_from_handler(dev->parent_bus, dev);
>        if (d) {
>            assert(!suffix);
>            s = d;
>        } else {
>            s = g_strdup(suffix);
>        }
>    } else {
>        s = g_strdup(suffix);
>    }
> }
> bootpath = g_strdup_printf("%s%s",
>                           devpath ? devpath : "",
>                           s ? s : "");
> g_free(devpath);
> g_free(s);
> return bootpath;
>
> +}
>
> /*
> This function returns null terminated string that consist of new line
> separated device paths.
>
> @@ -218,36 +251,10 @@ char *get_boot_devices_list(size_t *size)
>      bool ignore_suffixes = mc->ignore_boot_device_suffixes;
>  QTAILQ_FOREACH(i, &fw_boot_order, link) {
>    char *devpath = NULL,  *suffix = NULL;
>    char *bootpath;
>    char *d;
>    size_t len;
>
>    if (i->dev) {
>        devpath = qdev_get_fw_dev_path(i->dev);
>        assert(devpath);
>    }
>    if (!ignore_suffixes) {
>        if (i->dev) {
>            d = qdev_get_own_fw_dev_path_from_handler(i->dev->parent_bus,
>                                                      i->dev);
>            if (d) {
>                assert(!i->suffix);
>                suffix = d;
>            } else {
>                suffix = g_strdup(i->suffix);
>            }
>        } else {
>            suffix = g_strdup(i->suffix);
>        }
>    }
>    bootpath = g_strdup_printf("%s%s",
>                               devpath ? devpath : "",
>                               suffix ? suffix : "");
>    g_free(devpath);
>    g_free(suffix);
>    bootpath = get_boot_device_path(i->dev, ignore_suffixes, i->suffix);
>
>    if (total) {
>        list[total-1] = '\n';