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
