Thank you.
I will make this change.
As we discussed this will catch the case where blanks are in the target
name.
Joe
Jan Damborsky wrote:
> Hi Joe,
>
> the changes look good.
>
> I have only nit:
>
> setup-image.sh
> --------------
>
> 219 dirname_target="`dirname ${target}`"
> ->
> 219 dirname_target=`dirname "${target}"`
>
>
> 220 basename_target="`basename ${target}`"
> ->
> 220 basename_target=`basename "${target}"`
>
>
> Thank you,
> Jan
>
>
> 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.
>>
>>
>>
>>
>>
>>
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> caiman-discuss mailing list
>> caiman-discuss at opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>
