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
>
