Joe,
Thanks for your help and patience.
I have finally updated the webrev for this bug. I now have the
scripts setting the PATH appropriately. The scripts now contain
the line:
PATH=/usr/bin:/usr/sbin:/sbin:/usr/lib/installadm:${PATH}; export PATH
The new webrev is located at:
http://cr.opensolaris.org/~johnfisc/path-7325/
Thanks,
John
Joseph J. VLcek wrote:
> John Fischer wrote:
>> Joe,
>>
>> Thanks for taking the time to review the webrev.
>>
>> It is fairly simple to add the path this way. I think that the
>> best thing to do is have a standard of how the underlying scripts
>> are written. I am bent towards always full qualifying every
>> command from my security days in the desktop.
>>
>> If the scripts will be used directly from the command line or if
>> another fork/exec mechanism is developed within the installadm command
>> then you are absolutely correct in saying that there will be additional
>> problems.
>>
>> Looking at the scripts I see the following:
>>
>> 1) VARIABLE=<some binary>
>> ...
>> $VARIABLE ...
>>
>> i.e., GREP=/usr/bin/grep
>> $GREP "something" /etc/hosts
>>
>> 2) <some binary> # No PATH set
>>
>> i.e., grep "something" /etc/hosts
>>
>> 3) <some binary> # PATH set
>>
>> i.e., PATH=/usr/bin:/usr/sbin:/sbin:$PATH
>> grep "something" /etc/hosts
>>
>> 4) <fully path-ed binary>
>>
>> i.e., /usr/bin/grep "something" /etc/hosts
>>
>> The problem is that I see all 4 of these being used within the
>> scripts (actually sometimes all 4 in the same script) which is
>> probably from different developer's styles.
>>
>> Adding the PATH to all the scripts is also a simple solution and
>> could become the standard way we write scripts within the
>> component.
>>
>> Are you recommending that I modify the solution to add PATH to
>> each of the scripts?
>>
>> Thanks,
>>
>> John
>>
>> Joseph J. VLcek wrote:
>>> John Fischer wrote:
>>>> All,
>>>>
>>>> Fairly straight forward fix to a PATH issue. Low hanging fruit for the
>>>> new guy. More exciting things to come next week. ;-)
>>>>
>>>> webrev:
>>>>
>>>> http://cr.opensolaris.org/~johnfisc/path-7325/
>>>>
>>>> Open Solaris defect:
>>>>
>>>> 7325 - installadm script should set PATH
>>>>
>>>> The fix is to use putenv() of a sensible path which is /usr/bin,
>>>> /usr/sbin and /usr/lib/installadm. Only set once for the process
>>>> via an initialize variable.
>>>>
>>>> When someone has a chance I would appreciate a review.
>>>>
>>>> Thanks,
>>>>
>>>> John
>>>> _______________________________________________
>>>> caiman-discuss mailing list
>>>> caiman-discuss at opensolaris.org
>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>> John,
>>>
>>> I think having the installadm_system() function in
>>> usr/src/cmd/installadm/installadm_util.c export a PATH is an simple
>>> and elegant solution. However I think it might be safer in the long
>>> run to have each script define PATH.
>>>
>>> This would make each script more robust and able to stand on it's own
>>> without the requirement it be invoked by installadm_system().
>>>
>>> My concern is what happens if in the future a code change is made to
>>> have the scripts no longer invoked by installadm_system().
>>>
>>> What do you think?
>>>
>>> Joe
>>>
>>>
>>> installadm_system
>>>
> Hey John,
>
> Yes I would suggest adding PATH to each script.
>
> There is already a standard but as you point out it hasn't alway been
> followed.
>
> The standard can be seen here:
> http://hub.opensolaris.org/bin/view/Community+Group+on/shellstyle
>
> Which describes: There are two acceptable ways to do this:
> (1) make *all* command references in your script use explicit pathnames
> or (2) explicitly reset $PATH in your script.
>
> This is also documented here:
> http://installzone-wiki.central.sun.com/wiki/index.php/Ksh93_Tips
>
> and here:
> http://hub.opensolaris.org/bin/view/Project+shell/shellstyle#always_set_path
>
>
> As you point out, it is true that our scripts don't always follow this
> and sometimes mix methods. However I think when possible we should try
> to have our code adhere to this method. Which would lead us to have PATH
> defined in each script or alway use full path names.
>
> Some scripts indirectly use full path names with the CMD=<full path>;
> $CMD method.
>
> Joe
>
>
