[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [OSSTEST Nested PATCH v11 2/7] Parsing grub which has 'submenu' primitive
longtao.pang writes ("[OSSTEST Nested PATCH v11 2/7] Parsing grub which has 'submenu' primitive"): > Now auto-gen kernel grub2 config file's boot menu entries can have > 2-level hierarchy, containing 'submenu' primitive, which is comprised by > several sub-menuentries. Xen boot entries are grouped into such kind of > 'submenu' block. This patch adds setupboot_grub2() ability to handle > such new grub.cfg format Parsing submenus is good, but I have some style quibbles with your patch I'm afraid. > while (<$f>) { > next if m/^\s*\#/ || !m/\S/; > if (m/^\s*\}\s*$/) { > - die unless $entry; > + die unless $entry || $submenu; > + if(!defined $entry && defined $submenu){ $entry and $submenu are either some ref, or undef, so you don't need `defined'. You can just use them as if they were booleans in the if, just as you did above. (I'm not sure that `!defined $entry' adds much given the previous line, but that's maybe a matter or taste.) Missing spaces after if and inside `){'. > + logm("Met end of a submenu starting from ". > + "$submenu->{StartLine}. ". > + "Our want kern is $want_kernver"); > + $submenu=undef; Missing space after or around `='. (Other places too.) > + pop @offsets; > + $offsets[$#offsets]++; > + next; > + } > my (@missing) = > grep { !defined $entry->{$_} } > (defined $xenhopt > @@ -438,8 +448,12 @@ sub setupboot_grub2 ($$$$) { > } > if (m/^menuentry\s+[\'\"](.*)[\'\"].*\{\s*$/) { > die $entry->{StartLine} if $entry; > - $entry= { Title => $1, StartLine => $., Number => $count }; > - $count++; > + $entry= { Title => $1, StartLine => $., MenuEntryPath => > join ">", @offsets }; Needs rewrapping. Probably best to expand the { } into a multi-line structure. > + $offsets[$#offsets]++; > + } > + if (m/^submenu\s+[\'\"](.*)[\'\"].*\{\s*$/) { > + $submenu={ StartLine =>$., MenuEntryPath => join ">", > @offsets }; Unless I'm mistaken, the MenuEntryPath of a $submenu is never used ? Not setting it would avoid (a) a need to rewrap and (b) me complaining that you have open-coded the join twice. Thanks, Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |