Hi Joe.
Changes look pretty good. One thing I would change deals with checking
for string length of 1. From your reply below, the purpose of this
check is to catch the particular string "/". This is what I would check
for, not a strlen of 1. "a" is also a string of length 1 as well, and
would get different treatment than "aa" because of its length, but
that's not what you want.
So, I would change line 523 to be
if (str == "/")
The rest looks fine to me.
Thanks,
Jack
Joseph J VLcek wrote:
> Change made and tested. webrev updated.
>
> http://cr.opensolaris.org/~joev/bug7807/
>
> Joe
>
> 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.
>>
>> Good Point! Thanks for catching that.
>>
>> "/" should not be stripped by strip_ending_slashes
>>
>> Currently if one were to pass "/" to installadm create-service the
>> following error message is displayed: "Target directory is not empty"
>>
>> This makes sense to me.
>>
>> So I will update strip_ending_slashes to handle "/" as a special case.
>>
>> Thanks!
>> Joe
>>
>>
>>>
>>>
>>> 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
>>
>> _______________________________________________
>> caiman-discuss mailing list
>> caiman-discuss at opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
