Mailing List Archive

On Wed, 27 Dec 2000, you wrote:
> Hello, List !
> Looks like 'Latest CVS problem' became my destiny ;))
> This time it was very easy -
> drbd_common line 291 (have_command() function)
> Original :
> if ! `which $NAME 2>/dev/null` ; then (missing square brackets)
> Should be (hope everybody agree with me):
> if [ ! `which $NAME 2>/dev/null` ] ; then

Hi,

please have a look at my posting from Mon, Dec 18. (Latest CVS problem)

It should be
if ! which $NAME 1>/dev/null ; then

By the way, some people pointed out that this may lead to security problems, so
I think a solution without "which" and with full path should be more secure.

Can anyone tell me where fuser should be? On my system it's installed under
/usr/bin/fuser. Is this the correct location ?

Bye,
Sven

Oops, sorry ...
I've missed it.
And yes, from security point of view full path is preferred way, and we can use
just test -x checking for it.
But actually, who will exploit such a tiny hole except root itself ? Just do 'chmod
go-w drbd*'

Sven Sunder wrote:

> On Wed, 27 Dec 2000, you wrote:
> > Hello, List !
> > Looks like 'Latest CVS problem' became my destiny ;))
> > This time it was very easy -
> > drbd_common line 291 (have_command() function)
> > Original :
> > if ! `which $NAME 2>/dev/null` ; then (missing square brackets)
> > Should be (hope everybody agree with me):
> > if [ ! `which $NAME 2>/dev/null` ] ; then
>
> Hi,
>
> please have a look at my posting from Mon, Dec 18. (Latest CVS problem)
>
> It should be
> if ! which $NAME 1>/dev/null ; then
>
> By the way, some people pointed out that this may lead to security problems, so
> I think a solution without "which" and with full path should be more secure.
>
> Can anyone tell me where fuser should be? On my system it's installed under
> /usr/bin/fuser. Is this the correct location ?
>
> Bye,
> Sven

>
>
> Even more off topic, i'd like to get away from using internal ideas of
> tests.. have_file(), have_command(), etc.
>
> if have_file /foo/bar/baz; ...
>
> certainly can be replaced with a simple

I think you have to check not only one file, but actually five, and having a
function for it will save your
script size (or do you prefer to write five similar statements one by one ?)

>
>
> if [ -f /foo/bar/baz ]; ...

I think -x is more correct test in our case since we are looking for
_executable_ files

> which, unless I'm really missing something, is going to be a lot clearer.
>
> -josh
>
> _______________________________________________
> DRBD-devel mailing list
> DRBD-devel@example.com
> http://lists.sourceforge.net/mailman/listinfo/drbd-devel

* Sven Sunder (Cheru@example.com) [001227 12:35]:
> On Wed, 27 Dec 2000, you wrote:
> > Hello, List !
> > Looks like 'Latest CVS problem' became my destiny ;))
> > This time it was very easy -
> > drbd_common line 291 (have_command() function)
> > Original :
> > if ! `which $NAME 2>/dev/null` ; then (missing square brackets)
> > Should be (hope everybody agree with me):
> > if [ ! `which $NAME 2>/dev/null` ] ; then
>
> Hi,
>
> please have a look at my posting from Mon, Dec 18. (Latest CVS problem)
>
> It should be
> if ! which $NAME 1>/dev/null ; then

Agreed, in the case where you want to run a command as a test, it should
be done as you say.

> By the way, some people pointed out that this may lead to security
> problems, so I think a solution without "which" and with full path
> should be more secure.
>
> Can anyone tell me where fuser should be? On my system it's installed
> under /usr/bin/fuser. Is this the correct location ?

On my system 'fuser' is in /bin/fuser. Clearly this location will not be
uniform across all linuxen. I believe it should be reasonable to do one
of two things (correct me if I'm wrong all!)

1) Defined a minimal 'safe' path explicitly in the script such as:

PATH=/bin:/usr/bin

and then allow PATH to do what you expect.

As a side issue: I do not think it is wise, however, to assume that
'which' is a command that does what you want. I'd explicitly
'unset -v which; unset -f which; unalias which' if you really need it.

You could also test for fuser by using it (it always will find
itself..)

if fuser / >/dev/null 2>&1; then
we_have_fuser=true
fi

2) Manually scan among the possible locations:

if [ -x /usr/bin/fuser ];
...
elif [ -x /bin/fuser ];

1 is simpler of course, and thus I'd probably do that unless there is
objection from others.


Even more off topic, i'd like to get away from using internal ideas of
tests.. have_file(), have_command(), etc.

if have_file /foo/bar/baz; ...

certainly can be replaced with a simple

if [ -f /foo/bar/baz ]; ...

which, unless I'm really missing something, is going to be a lot clearer.

-josh

* Dim Zegebart (zager@example.com) [001227 14:08]:
> >
> >
> > Even more off topic, i'd like to get away from using internal ideas of
> > tests.. have_file(), have_command(), etc.
> >
> > if have_file /foo/bar/baz; ...
> >
> > certainly can be replaced with a simple
>
> I think you have to check not only one file, but actually five, and
> having a function for it will save your script size (or do you prefer to
> write five similar statements one by one ?)

I prefer to have no subroutine at all when the equivalent in-line code is
just as terse and in fact the normal way to do it. That was the point.

Creating aditional interfaces which add no functionality and merely
obfuscate what is going on is not beneficial.


> > if [ -f /foo/bar/baz ]; ...
>
> I think -x is more correct test in our case since we are looking for
> _executable_ files

This was in the case of the routine have_file, as opposed to have_command,
which would of course use -x or so.

-josh

> > I think you have to check not only one file, but actually five, and
> > having a function for it will save your script size (or do you prefer to
> > write five similar statements one by one ?)
>
> I prefer to have no subroutine at all when the equivalent in-line code is
> just as terse and in fact the normal way to do it. That was the point.

We you fix it once you fix it all. If the test in my case -f is not
appropritate but need a -x I will only have to cahnge it once.

> Creating aditional interfaces which add no functionality and merely
> obfuscate what is going on is not beneficial.

As well have_file and have_exec are more easy too read for someone not used
with the code..
I don't think it is obfuscating ...

I admit my eval stuff to parse the file is evil and must be removed but we I
wrote it, it was for my own use mainly.
If now other person wants to contrubute it will have to go ..

I am just waiting for the new parsing code to be included in the CVS ...

> > I think -x is more correct test in our case since we are looking for
> > _executable_ files

You are right I must be fixed too ....
Man, I hope I will not forget all the thing I have to do ...

Thomas