Jack Schwartz wrote: > 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 >
Hey Jack, Thanks for making the code changes. As we discussed on the phone. ----------------------------- I do not like the strdup approach because it leaves unfreed memory. I would like to see this function modify the string passed in and document that. If the caller wants the initial string preserved they need to dup it prior to the invocation of strip_ending_slashes() Another option is for you free the memory someplace after the call to strip_ending_slashes. Joe
