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
