Jan Damborsky wrote: > Dave, > > > Dave Miner wrote: >> jan damborsky wrote: >>> Hi Dave, >>> >>> based on the yesterday's discussion during Caiman meeting, >>> I am assuming that following bug is to be qualified as >>> 2008.11 stopper. This fix addresses scenario Rafal >>> reported in bug 4872. >>> >>> Could you please let me know, if my understanding is correct ? >>> >> Yes, I've approved it. > > Thank you > >>> If this is the case, could I please ask two people to review >>> those changes ? >>> >>> 4980 Installer should convert legacy Solaris partition (0x82) to >>> Solaris2 one (0xbf) >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=4980 >>> >>> The webrev is available at: >>> http://cr.opensolaris.org/~dambi/bug-4980 >>> >> 131: s/SUNW/SUN/ > > Done. > >> >> 136: s/one// > > Done. > >> >> 153: it's not at all obvious to me why "0" is the right value to use >> here for the handle; perhaps a comment, at least. > > 'handle' parameter is currently unused > in this function - I have put appropriate > comment there explaining this. > >> I also would have expected to check the return value on this call, >> not just whether committed_disk_target == NULL; that's not explicitly >> guaranteed to be its state in error cases, though I agree that it is >> in the current implementation. > > This is a good suggestion - I have modified > the code, so that it checks for return value. > Since om_set_disk_partition_info() returns > OM_FAILURE in all cases when committed_disk_target > is set to NULL, 'committed_disk_target == NULL' > check became redundant and was removed. > > Updated webrev with all changes has been published > at the same location. >
Thanks, looks good. Dave
