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.





-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: remove_consecutive_n_trailing_slashes.sh
URL: 
<http://mail.opensolaris.org/pipermail/caiman-discuss/attachments/20090407/353f6b18/attachment.ksh>

Reply via email to