grub-devel
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]