[PATCH V2] ui/ncurses: Add device hierarchy

Sam Mendoza-Jonas sam.mj at au1.ibm.com
Wed Jul 16 13:39:49 EST 2014


On 16/07/14 13:23, Jeremy Kerr wrote:
> Hi Sam,
>
>> Boot options are now listed under their matching boot device in the
>> ncurses UI to help differentitate similar boot option names
>
> I get the following build failure with this patch:
>
> ../../../ui/ncurses/nc-menu.c: In function 'pmenu_find_device':
> ../../../ui/ncurses/nc-menu.c:225:24: error: 'bd' may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
>    struct blockdev_info *bd;
>                          ^
> ../../../ui/ncurses/nc-menu.c:277:10: error: 'intf' may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
>     mac_str(intf->hwaddr, intf->hwaddr_size,
>            ^
> cc1: all warnings being treated as errors
>
> - I think we'll need a check for matched in both of those cases.

Eek! Yep, that was it.

....

>> +	/* All actual boot entries are 'tabbed' across */
>> +	shift = talloc_asprintf(cui->main, "%s%s", tab, opt->name ? : "Unknown Name");
>
> 80 chars again :)  (also in a couple of other places)
>
> Not sure why this var is called 'shift' - something from the previous
> version?

Just a name that made sense to me when I was writing it, I'll make it 
more readable! :) Just where we add some whitespace before the names of
normal boot entries.

...

>
>> +
>> +	if (found) {
>
> We'll need to initialise this to false, no?
>

Yep

...
>
> Would it be worth adding a device pointer to item->data?  We're doing a
> few strcmps around the place.
>
...
>
> Looks okay, but makes me think we could have a single representation of
> a bootable device, with a name, type and ID. This would help out with
> the 'boot a specific device' code I'm working on too.

Yes, in hindsight I could simplify this more. If your code doesn't 
depend directly on anything here, I'll leave it as "v1" for the moment 
and then improve it to help it line up more closely with the
specific device stuff.
Thanks for the review!

>
> However, this would be something for a later change, I'm happy to use
> the blockdev and interface arrays as they are for this code.
>
> Thanks!
>
>
> Jeremy
>


-- 
-----------
LTC Ozlabs
IBM



More information about the Petitboot mailing list