Re: [PATCH] pygrub: fix non-interactive parsing of grub1 config files

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Nov 04, 2014 at 10:46:37AM +0000, Ian Campbell wrote:
> On Mon, 2014-10-27 at 12:35 -0400, Boris Ostrovsky wrote:
> > On 10/27/2014 12:00 PM, Simon Rowe wrote:
> > > Changes to handle non-numeric default attributes for grub2 caused run_grub()
> > > to attempt to index into the images list using a string. Pull out the code
> > > that handles submenus into a new function and use that to ensure sel is
> > > numeric.
> > >
> > > Reported-by: David Scott <dave.scott@citrix.com>
> > > Signed-off-by: Simon Rowe <simon.rowe@eu.citrix.com>
> > 
> > Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> And applied as a bug fix (I'm not sure if Konrad's response was intended
> as a release-ack).

Yes. Thank you for checking this patch in.
> 
> > 
> > > ---
> > >   tools/pygrub/src/pygrub |   23 ++++++++++++++---------
> > >   1 file changed, 14 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub
> > > index 1ae34a2..4cc846f 100644
> > > --- a/tools/pygrub/src/pygrub
> > > +++ b/tools/pygrub/src/pygrub
> > > @@ -456,11 +456,9 @@ class Grub:
> > >           del f
> > >           self.cf.parse(buf)
> > >   
> > > -    def run(self):
> > > -        timeout = int(self.cf.timeout)
> > > -
> > > +    def image_index(self):
> > >           if self.cf.default.isdigit():
> > > -            self.selected_image = int(self.cf.default)
> > > +            sel = int(self.cf.default)
> > >           else:
> > >               # We don't fully support submenus. Look for the leaf value in
> > >               # "submenu0>submenu1>...>menuentry" and hope that it's unique.
> > > @@ -472,16 +470,23 @@ class Grub:
> > >                       break
> > >   
> > >               # Map string to index in images array
> > > -            self.selected_image = 0
> > > +            sel = 0
> > >               for i in range(len(self.cf.images)):
> > >                   if self.cf.images[i].title == title:
> > > -                    self.selected_image = i
> > > +                    sel = i
> > >                       break
> > >   
> > >           # If the selected (default) image doesn't exist we select the first entry
> > > -        if self.selected_image > len(self.cf.images):
> > > +        if sel > len(self.cf.images):
> > >               logging.warning("Default image not found")
> > > -            self.selected_image = 0
> > > +            sel = 0
> > > +
> > > +        return sel
> > > +
> > > +    def run(self):
> > > +        timeout = int(self.cf.timeout)
> > > +        self.selected_image = self.image_index()
> > > +
> > >           self.isdone = False
> > >           while not self.isdone:
> > >               self.run_main(timeout)
> > > @@ -626,7 +631,7 @@ def run_grub(file, entry, fs, cfg_args):
> > >       if interactive and not list_entries:
> > >           curses.wrapper(run_main)
> > >       else:
> > > -        sel = g.cf.default
> > > +        sel = g.image_index()
> > >   
> > >       # set the entry to boot as requested
> > >       if entry is not None:
> > 
> 
>