On 03/26/09 13:58, Joseph J VLcek wrote:
> 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()
OK. Changed.
Same webrev location.
Please do the needful.
Thanks,
Jack
>
> Another option is for you free the memory someplace after the call to
> strip_ending_slashes.
>
> Joe
