[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] ieee1275: split up grub_machine_get_bootlocation
From: |
Daniel Kiper |
Subject: |
Re: [PATCH] ieee1275: split up grub_machine_get_bootlocation |
Date: |
Thu, 8 Mar 2018 17:02:55 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Thu, Mar 08, 2018 at 08:42:14AM -0700, Eric Snowberg wrote:
>
> > On Mar 8, 2018, at 4:19 AM, Daniel Kiper <address@hidden> wrote:
> >
> > On Wed, Mar 07, 2018 at 01:48:10PM -0800, Eric Snowberg wrote:
> >> Split up some of the functionality in grub_machine_get_bootlocation into
> >> grub_ieee1275_get_boot_dev. This will allow for code reuse in a follow on
> >> patch.
> >
> > I would like to see follow up patches in such cases like that one.
>
> Here is the follow up patch I’m preparing to send that will use it (look at
> the add_bootpath function):
>
> https://github.com/esnowberg/grub2-sparc/commit/94e4e64c98bce8f43ced440928b3d1e83b36ed11.patch
>
> This patch also uses the other additions I’ve sent recently.
>
> >
> >> Signed-off-by: Eric Snowberg <address@hidden>
> >> ---
> >> grub-core/kern/ieee1275/init.c | 24 ++++--------------------
> >> grub-core/kern/ieee1275/openfw.c | 27 +++++++++++++++++++++++++++
> >> include/grub/ieee1275/ieee1275.h | 1 +
> >> 3 files changed, 32 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/grub-core/kern/ieee1275/init.c
> >> b/grub-core/kern/ieee1275/init.c
> >> index 1259022..dff2184 100644
> >> --- a/grub-core/kern/ieee1275/init.c
> >> +++ b/grub-core/kern/ieee1275/init.c
> >> @@ -93,29 +93,13 @@ void (*grub_ieee1275_net_config) (const char *dev,
> >> char **device, char **path,
> >> void
> >> grub_machine_get_bootlocation (char **device, char **path)
> >> {
> >> - char *bootpath;
> >> - grub_ssize_t bootpath_size;
> >> + char *bootpath = NULL;
> >
> > I do not think that you need initialize this if…
>
> I thought the compiler complained about that, but I’ll try to remove it.
>
> >
> >> char *filename;
> >> char *type;
> >>
> >> - if (grub_ieee1275_get_property_length (grub_ieee1275_chosen, "bootpath",
> >> - &bootpath_size)
> >> - || bootpath_size <= 0)
> >> - {
> >> - /* Should never happen. */
> >> - grub_printf ("/chosen/bootpath property missing!\n");
> >> - return;
> >> - }
> >> -
> >> - bootpath = (char *) grub_malloc ((grub_size_t) bootpath_size + 64);
> >> - if (! bootpath)
> >> - {
> >> - grub_print_error ();
> >> - return;
> >> - }
> >> - grub_ieee1275_get_property (grub_ieee1275_chosen, "bootpath", bootpath,
> >> - (grub_size_t) bootpath_size + 1, 0);
> >> - bootpath[bootpath_size] = '\0';
> >> + grub_ieee1275_get_boot_dev (&bootpath);
> >> + if (bootpath == NULL)
> >
> > if (! bootpath)
>
> I’ll make this change.
>
> >
> >> + return;
> >>
> >> /* Transform an OF device path to a GRUB path. */
> >>
> >> diff --git a/grub-core/kern/ieee1275/openfw.c
> >> b/grub-core/kern/ieee1275/openfw.c
> >> index ddb7783..81c03cf 100644
> >> --- a/grub-core/kern/ieee1275/openfw.c
> >> +++ b/grub-core/kern/ieee1275/openfw.c
> >> @@ -561,3 +561,30 @@ grub_ieee1275_canonicalise_devname (const char *path)
> >> return NULL;
> >> }
> >>
> >> +void
> >> +grub_ieee1275_get_boot_dev (char **boot_dev)
> >
> > ...you return char * here…
>
> The returned value is in the boot_dev. Just to clarify, you want me to
> change the function prototype to return char * too?
Sorry, I was not clear enough. I though about:
char *
grub_ieee1275_get_boot_dev (void)
{
...
}
Daniel