Evan Layton wrote: > Joseph J. VLcek wrote: >> Evan Layton wrote: >>> Joseph J. VLcek wrote: >>>> Evan Layton wrote: >>>>> I need to get a review the following simple fix. >>>>> >>>>> This problem was caused by the inappropriate use of strncmp() in >>>>> my fix for 3332. Because of the way strncmp() was being used we >>>>> end up comparing only the length of the currently active BE name. >>>>> What this causes is if the current BE is named say snv_108 and the >>>>> BE we are attempting to rename is called snv_108-1 we only check >>>>> the length of the current BE and of course it says these are the >>>>> same so the rename fails. The fix in this case is to simply >>>>> replace the strncmp() with strcmp(). >>>>> >>>>> 7269 beadm rename can fail. >>>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=7269 >>>>> >>>>> Webrev: >>>>> http://cr.opensolaris.org/~evanl/7269/ >>>>> >>>>> Thanks! >>>>> -evan >>>>> _______________________________________________ >>>>> caiman-discuss mailing list >>>>> caiman-discuss at opensolaris.org >>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>>> >>>> I have a safer suggestion: >>>> >>>> Maybe it would be safest to continue using strncmp with the length >>>> of the longest string passed as "n" (lenght). >>>> >>>> >>>> >>>> e.g.: >>>> #define MAX(a, b) ((a) > (b) ? (a) : (b)) >>>> >>>> ... >>>> if (strncmp(bt.obe_name, cbt.obe_name, >>>> MAX(strlen(bt.obe_name), strlen(cbt.obe_name))) == 0) { >>>> >>>> ... >>>> >>>> What do you think? Would that be safer or unnecessary? >>>> >>>> Joe >>>> >>> >>> I thought of something like this as well be decided against it sine >>> the name of the currently active BE is already known to be a valid >>> name it's length is fine. If the name we are attempting to use is >>> too long it won't matter because we'll check it against the current >>> BE and it will at most stop checking at the end of the currently >>> active BE name. If they don't match the new BE name is checked for >>> validity just after this check. >>> >>> I can certainly add this check for max length but I didn't think it >>> was necessary. >>> >>> -evan >> Fair enough. > > And then I changed my mind and did what you suggested anyway. :-) > > Webrev is updated. > > -evan Would you make up "my" mind! ;) ;) ;)
Looks good to me now. Joe
