Evan, 355 - Shouldn't this be set to 0 ?
386 - Instead of setting num_lines = num_tmp_lines here, initialize it to 0, and use (and increment) it as the index to entries[] at lines 392 and 399. 431, 451 - If entering these if() clauses, you need to check if entries[] if populated, and if so, free it. (I would suggest setting a ret code, and then jumping down to a cleanup section below.) thanks, -ethan Evan Layton wrote: > Hi Ethan, > > I've corrected these two issues, retested and updated the webrev. The code > now > does the collection of the menu entry lines at the point we detect that we've > found the original BE's entry. Once we have that information we continue > through > the file making sure we don't also have a duplicate entry. Finally we collect > the menu entry lline and maintain their order so that the new BE's menu entry > will also match the order of the entry we're copied from. > > Thanks! > -evan > > Ethan Quach wrote: > >> Evan, >> >> The existing code accounts for the case where if the new BE we're adding >> already has an entry in the menu.lst, it does the proper thing. However, >> with your changes, it might fail to detect that. For the case where the >> new >> BE already has an entry but is below orig_root_ds entry, you no longer >> find it. >> >> >> Is it possible for a line to exist be between the 'title' line and >> 'bootfs'? >> If so, that line would have been recorded in entries[] and written out >> after the 'bootfs' line for the new BE. I don't know if that would cause >> a problem but just something to note. >> >> >> thanks, >> -ethan >> >> >> Evan Layton wrote: >> >>> I need to get a review the changes for >>> >>> 3654 libbe should keep menu.lst customizations when cloning >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=3654 >>> >>> The webrev is available from: >>> http://cr.opensolaris.org/~evanl/webrev.3654/ >>> >>> This fix changes the way we build grub menu entries. When we're >>> creating a new BE we take a look at the grub entry lines from the >>> parent of BE we're creating and we edit the title and bootfs lines >>> the rest of the lines are copied directly form the parents entry. >>> >>> This has been tested using "beadm create" and "beadm activate" which >>> call into be_copy() and be_activate() which are the only consumers >>> of this functionality. I've also verified that changed entries in >>> menu.lst are copied from the parent BE to the entry for the >>> clone/copy. >>> >>> These changes also include some simple lint clean-up. >>> >>> Thanks, >>> -evan >>> _______________________________________________ >>> caiman-discuss mailing list >>> caiman-discuss at opensolaris.org >>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>> >>> > > _______________________________________________ > caiman-discuss mailing list > caiman-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >
