Hi Joe. Thanks for reviewing. Please see my comments below.
On 03/26/09 13:17, Joseph J VLcek wrote: > Jack Schwartz wrote: >> Hi everyone. >> >> Please review a simple fix to remove trailing slashes from installadm >> create-service target directories. >> >> Bug: http://defect.opensolaris.org/bz/show_bug.cgi?id=3773 >> >> webrev: http://cr.opensolaris.org/~schwartz/090326.1/webrev/ >> >> Tested by providing installadm create-service commands with target >> dirs containing 0-2 trailing slashes. >> >> Thanks, >> Jack >> _______________________________________________ >> caiman-discuss mailing list >> caiman-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss > > Looks good Jack. > > As we discussed on the phone. > > Please add a function description including the 2 boundary cases we > discussed. That being: > > Blank spaces at the end of the line are "not" ignored. So it a blank > space follows any trailing slashes the slashes will not be removed. Done. > > If a one character string containing a slash is passed in the result > with be an empty string. Done. > > And as you pointed out it would make more sense to put this in > installadm_utils.c: Done. I made one other change as well: I thought it would be more consistent to dup the string whether or not it was empty. This also allowed me to get rid of a (len==0) check and the len variable, so it's shorter. I have retested with all my test cases of before, plus / and an empty string. Please have another look, and then bless or comment as you see fit. Thanks again, Jack > > usr/src/cmd/installadm/installadm_util.c > > Joe
