Ethan Quach wrote:
> Joe,
>
> This is just a nit - since a trailing slash is appended to
> $dirname_target at line 224, then for cases where the
> code enters the else at 232, wouldn't you be setting
> $target with a value with a trailing slash again?
>
> Looks like it work if you just removed the chunk from
> 222-225 all together, and change 230 to be:
>
> target="${dirname_target}/${basename_target}"
This will result in //a returning //a
But as we discussed on the phone the flow of the logic is confusing. I
will rework it to reflect our discussion.
Thank you!
Joe
>
>
> thanks,
> -ethan
>
>
> Joseph J VLcek wrote:
>> Ethan Quach wrote:
>>> Hi Joe,
>>>
>>> The changes look okay, but seems like it implies that the
>>> target_directory argument passed into create-service
>>> cannot be the root dir, "/".
>>>
>>> Either this should be a) allowed, if its legal, or b) checked
>>> as a special case, and told to the user with a specific error
>>> message. As it stands now, it seems we simply spit out the
>>> usage when "/" is passed in, and that doesn't really tell the
>>> user what's going on if its not legal.
>>>
>>>
>>> thanks,
>>> -ethan
>>>
>>>
>>> Joseph J VLcek wrote:
>>>> Could I please get a code review for a small change to fix bug 7807 ?
>>>>
>>>> Bug:
>>>> -----
>>>> 7807 installadm create-service crashes with no args
>>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=7807
>>>>
>>>>
>>>> Webrev:
>>>> -------
>>>> http://cr.opensolaris.org/~joev/bug7807/
>>>>
>>>>
>>>> The modules affected and tested:
>>>> --------------------------------
>>>> installadm
>>>>
>>>> Testing done:
>>>> --------------
>>>>
>>>> 1.
>>>> I built a stand alone C program to exercise the changed function,
>>>> strip_ending_slashes. The stand along program passed various strings
>>>> including NULL and the empty string to strip_ending_slashes.
>>>>
>>>> 2.
>>>> I built and ran installadm create-service on x86 with both no
>>>> arguments and valid arguments.
>>>>
>>>>
>>>>
>>>> Thank you!
>>>>
>>>> Joe
>>>> _______________________________________________
>>>> caiman-discuss mailing list
>>>> caiman-discuss at opensolaris.org
>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>
>>
>> Please review the updated webrev for bug 7807.
>>
>> Updated webrev:
>> ---------------
>> http://cr.opensolaris.org/~joev/bug7807/
>>
>> After discussing this with Ethan I am taking a different approach to
>> resolve this.
>>
>> I am reworking the solution to bug 3773 - installadm output ln link
>> error message
>>
>> Bug 3773 inadvertently lead to this bug.
>>
>> The solution is to not to define the new function
>> strip_ending_slashes() in installadm_utils.c
>>
>> Instead setup-image.sh will have code to remove consecutive and
>> trailing slashes from the directory specified by the user.
>>
>> I have tested the new logic, being added to setup-image.sh, using the
>> attached test script.
>>
>> I have also tested using installadm as I had done for the initial code
>> review request.
>>
>> Thank you! Joe.
>>
>>
>>
>>
>>
