[PATCH] Use full path to dnsdomainname and domainname in scripts/mkcompile_h

[PATCH] Use full path to dnsdomainname and domainname in scripts/mkcompile_h

[Glenn Sommer]

With reference to: http://bugzilla.kernel.org/show_bug.cgi?id=14920
I'll post my suggestion here.

Currently scripts/mkcompile_h checks for "/bin/dnsdomainname" and
"/bin/domainname" when trying to find the DNS name.
Though, when running the executable - the full path isn't used!

IMO if we check for "/bin/dnsdomainname", we should also use
"/bin/dnsdomainname" - and not blindly trust /bin is the first directory in
$PATH  which contains a executable named "dnsdomainname"


I propose to use the full path, that we know is valid. Here's my proposed patch:


--- scripts/mkcompile_h.orig	2009-12-28 23:02:34.000000000 +0100
+++ scripts/mkcompile_h	2009-12-28 23:03:12.000000000 +0100
@@ -66,9 +66,9 @@
   echo \#define LINUX_COMPILE_HOST \"`hostname | $UTS_TRUNCATE`\"

   if [ -x /bin/dnsdomainname ]; then
-    echo \#define LINUX_COMPILE_DOMAIN \"`dnsdomainname | $UTS_TRUNCATE`\"
+    echo \#define LINUX_COMPILE_DOMAIN \"`/bin/dnsdomainname | $UTS_TRUNCATE`\"
   elif [ -x /bin/domainname ]; then
-    echo \#define LINUX_COMPILE_DOMAIN \"`domainname | $UTS_TRUNCATE`\"
+    echo \#define LINUX_COMPILE_DOMAIN \"`/bin/domainname | $UTS_TRUNCATE`\"
   else
     echo \#define LINUX_COMPILE_DOMAIN
   fi


Signed-off-by: Glenn Sommer <glemsom@gmail.com>

Re: [PATCH] Use full path to dnsdomainname and domainname in scripts/mkcompile_h

[Américo Wang]

On Wed, Jan 20, 2010 at 2:29 AM, Glenn Sommer <glemsom@gmail.com> wrote:
> With reference to: http://bugzilla.kernel.org/show_bug.cgi?id=14920
> I'll post my suggestion here.
>
> Currently scripts/mkcompile_h checks for "/bin/dnsdomainname" and
> "/bin/domainname" when trying to find the DNS name.
> Though, when running the executable - the full path isn't used!
>
> IMO if we check for "/bin/dnsdomainname", we should also use
> "/bin/dnsdomainname" - and not blindly trust /bin is the first directory in
> $PATH  which contains a executable named "dnsdomainname"
>
>
> I propose to use the full path, that we know is valid. Here's my proposed patch:
>
>
> --- scripts/mkcompile_h.orig    2009-12-28 23:02:34.000000000 +0100
> +++ scripts/mkcompile_h 2009-12-28 23:03:12.000000000 +0100
> @@ -66,9 +66,9 @@
>   echo \#define LINUX_COMPILE_HOST \"`hostname | $UTS_TRUNCATE`\"
>
>   if [ -x /bin/dnsdomainname ]; then
> -    echo \#define LINUX_COMPILE_DOMAIN \"`dnsdomainname | $UTS_TRUNCATE`\"
> +    echo \#define LINUX_COMPILE_DOMAIN \"`/bin/dnsdomainname | $UTS_TRUNCATE`\"
>   elif [ -x /bin/domainname ]; then
> -    echo \#define LINUX_COMPILE_DOMAIN \"`domainname | $UTS_TRUNCATE`\"
> +    echo \#define LINUX_COMPILE_DOMAIN \"`/bin/domainname | $UTS_TRUNCATE`\"
>   else
>     echo \#define LINUX_COMPILE_DOMAIN
>   fi
>
>
> Signed-off-by: Glenn Sommer <glemsom@gmail.com>

Makes sense, but is that possible we have 'domainname' installed in two
different directories?

Re: [PATCH] Use full path to dnsdomainname and domainname in scripts/mkcompile_h

[Glenn Sommer]

2010/1/20 Américo Wang <xiyou.wangcong@gmail.com>:
> On Wed, Jan 20, 2010 at 2:29 AM, Glenn Sommer <glemsom@gmail.com> wrote:
>> With reference to: http://bugzilla.kernel.org/show_bug.cgi?id=14920
>> I'll post my suggestion here.
>>
>> Currently scripts/mkcompile_h checks for "/bin/dnsdomainname" and
>> "/bin/domainname" when trying to find the DNS name.
>> Though, when running the executable - the full path isn't used!
>>
>> IMO if we check for "/bin/dnsdomainname", we should also use
>> "/bin/dnsdomainname" - and not blindly trust /bin is the first directory in
>> $PATH  which contains a executable named "dnsdomainname"
>>
>>
>> I propose to use the full path, that we know is valid. Here's my proposed patch:
>>
>>
>> --- scripts/mkcompile_h.orig    2009-12-28 23:02:34.000000000 +0100
>> +++ scripts/mkcompile_h 2009-12-28 23:03:12.000000000 +0100
>> @@ -66,9 +66,9 @@
>>   echo \#define LINUX_COMPILE_HOST \"`hostname | $UTS_TRUNCATE`\"
>>
>>   if [ -x /bin/dnsdomainname ]; then
>> -    echo \#define LINUX_COMPILE_DOMAIN \"`dnsdomainname | $UTS_TRUNCATE`\"
>> +    echo \#define LINUX_COMPILE_DOMAIN \"`/bin/dnsdomainname | $UTS_TRUNCATE`\"
>>   elif [ -x /bin/domainname ]; then
>> -    echo \#define LINUX_COMPILE_DOMAIN \"`domainname | $UTS_TRUNCATE`\"
>> +    echo \#define LINUX_COMPILE_DOMAIN \"`/bin/domainname | $UTS_TRUNCATE`\"
>>   else
>>     echo \#define LINUX_COMPILE_DOMAIN
>>   fi
>>
>>
>> Signed-off-by: Glenn Sommer <glemsom@gmail.com>
>
> Makes sense, but is that possible we have 'domainname' installed in two
> different directories?
>

Usually "domainname" should be installed in /bin.
I'm just thinking if one does something like this:

* Place shellscript named "domainname" in /home/stupiduser/scripts
(This shellscript should output some text... Let's say
"my-stupid-shell-script")
* Set PATH=/home/stupiduser/scripts:$PATH
* Compile Linux kernel

Doing the above will result in scripts/mkcompile_h testing for
/bin/domainname, but actually using
/home/stupiduser/scripts/domainname - which is this case will output
something wrong.
One could argue it's your own fault then - and I agree! Doing the
above is stupid!

Anyway, if we test for the executable using a complete path - we
should also use that complete path when running the executable!


Alternatively, if we want it to be more flexible(and allow the above)
- we should do something like:

domainname_executable=`which domainname`
if [ ! -z "$domainname_executable" ] && [ -x "$domainname_executable" ]; then

Re: [PATCH] Use full path to dnsdomainname and domainname in scripts/mkcompile_h

[Américo Wang]

On Wed, Jan 20, 2010 at 11:06 PM, Glenn Sommer <glemsom@gmail.com> wrote:
> 2010/1/20 Américo Wang <xiyou.wangcong@gmail.com>:
>> On Wed, Jan 20, 2010 at 2:29 AM, Glenn Sommer <glemsom@gmail.com> wrote:
>>> With reference to: http://bugzilla.kernel.org/show_bug.cgi?id=14920
>>> I'll post my suggestion here.
>>>
>>> Currently scripts/mkcompile_h checks for "/bin/dnsdomainname" and
>>> "/bin/domainname" when trying to find the DNS name.
>>> Though, when running the executable - the full path isn't used!
>>>
>>> IMO if we check for "/bin/dnsdomainname", we should also use
>>> "/bin/dnsdomainname" - and not blindly trust /bin is the first directory in
>>> $PATH  which contains a executable named "dnsdomainname"
>>>
>>>
>>> I propose to use the full path, that we know is valid. Here's my proposed patch:
>>>
>>>
>>> --- scripts/mkcompile_h.orig    2009-12-28 23:02:34.000000000 +0100
>>> +++ scripts/mkcompile_h 2009-12-28 23:03:12.000000000 +0100
>>> @@ -66,9 +66,9 @@
>>>   echo \#define LINUX_COMPILE_HOST \"`hostname | $UTS_TRUNCATE`\"
>>>
>>>   if [ -x /bin/dnsdomainname ]; then
>>> -    echo \#define LINUX_COMPILE_DOMAIN \"`dnsdomainname | $UTS_TRUNCATE`\"
>>> +    echo \#define LINUX_COMPILE_DOMAIN \"`/bin/dnsdomainname | $UTS_TRUNCATE`\"
>>>   elif [ -x /bin/domainname ]; then
>>> -    echo \#define LINUX_COMPILE_DOMAIN \"`domainname | $UTS_TRUNCATE`\"
>>> +    echo \#define LINUX_COMPILE_DOMAIN \"`/bin/domainname | $UTS_TRUNCATE`\"
>>>   else
>>>     echo \#define LINUX_COMPILE_DOMAIN
>>>   fi
>>>
>>>
>>> Signed-off-by: Glenn Sommer <glemsom@gmail.com>
>>
>> Makes sense, but is that possible we have 'domainname' installed in two
>> different directories?
>>
>
> Usually "domainname" should be installed in /bin.
> I'm just thinking if one does something like this:
>
> * Place shellscript named "domainname" in /home/stupiduser/scripts
> (This shellscript should output some text... Let's say
> "my-stupid-shell-script")
> * Set PATH=/home/stupiduser/scripts:$PATH
> * Compile Linux kernel
>
> Doing the above will result in scripts/mkcompile_h testing for
> /bin/domainname, but actually using
> /home/stupiduser/scripts/domainname - which is this case will output
> something wrong.
> One could argue it's your own fault then - and I agree! Doing the
> above is stupid!
>
> Anyway, if we test for the executable using a complete path - we
> should also use that complete path when running the executable!
>
>
> Alternatively, if we want it to be more flexible(and allow the above)
> - we should do something like:
>
> domainname_executable=`which domainname`
> if [ ! -z "$domainname_executable" ] && [ -x "$domainname_executable" ]; then
>

Yeah, this seems better for me.
Thanks!

Re: [PATCH] Use full path to dnsdomainname and domainname in scripts/mkcompile_h

[Michal Marek]

On 26.1.2010 04:55, Américo Wang wrote:
> On Wed, Jan 20, 2010 at 11:06 PM, Glenn Sommer <glemsom@gmail.com> wrote:
>> Alternatively, if we want it to be more flexible(and allow the above)
>> - we should do something like:
>>
>> domainname_executable=`which domainname`
>> if [ ! -z "$domainname_executable" ] && [ -x "$domainname_executable" ]; then
>>

(or 'if command -v domainname >/dev/null 2>&1; then domainname ...')


> Yeah, this seems better for me.

Me too. Glenn, could you send a complete patch doing this? I'll add it
to the kbuild tree then.

Thanks,
Michal

Re: [PATCH] Use full path to dnsdomainname and domainname in scripts/mkcompile_h

[Glenn Sommer]

2010/1/26 Michal Marek <mmarek@suse.cz>:
> On 26.1.2010 04:55, Américo Wang wrote:
>> On Wed, Jan 20, 2010 at 11:06 PM, Glenn Sommer <glemsom@gmail.com> wrote:
>>> Alternatively, if we want it to be more flexible(and allow the above)
>>> - we should do something like:
>>>
>>> domainname_executable=`which domainname`
>>> if [ ! -z "$domainname_executable" ] && [ -x "$domainname_executable" ]; then
>>>
>
> (or 'if command -v domainname >/dev/null 2>&1; then domainname ...')
>
>
>> Yeah, this seems better for me.
>
> Me too. Glenn, could you send a complete patch doing this? I'll add it
> to the kbuild tree then.
>
> Thanks,
> Michal
>

Yeah, good idea with "command -v" ! :)
( note: `command -v` will return true if the executable is found -
else it will return false. )

mkcompile_h is changed slightly in 2.6.32. Here's my new proposed patch:


--- scripts/mkcompile_h.orig	2010-01-26 18:59:37.000000000 +0100
+++ scripts/mkcompile_h	2010-01-26 20:03:42.000000000 +0100
@@ -67,9 +67,9 @@
   echo \#define LINUX_COMPILE_BY \"`whoami`\"
   echo \#define LINUX_COMPILE_HOST \"`hostname | $UTS_TRUNCATE`\"

-  if [ -x /bin/dnsdomainname ]; then
+  if [ `command -v dnsdomainname 2> /dev/null` ]; then
     domain=`dnsdomainname 2> /dev/null`
-  elif [ -x /bin/domainname ]; then
+  elif [ `command -v domainname 2> /dev/null` ]; then
     domain=`domainname 2> /dev/null`
   fi

Signed-off-by: Glenn Sommer <glemsom@gmail.com>

Re: [PATCH] Use full path to dnsdomainname and domainname in scripts/mkcompile_h

[Américo Wang]

On Wed, Jan 27, 2010 at 3:10 AM, Glenn Sommer <glemsom@gmail.com> wrote:
> 2010/1/26 Michal Marek <mmarek@suse.cz>:
>> On 26.1.2010 04:55, Américo Wang wrote:
>>> On Wed, Jan 20, 2010 at 11:06 PM, Glenn Sommer <glemsom@gmail.com> wrote:
>>>> Alternatively, if we want it to be more flexible(and allow the above)
>>>> - we should do something like:
>>>>
>>>> domainname_executable=`which domainname`
>>>> if [ ! -z "$domainname_executable" ] && [ -x "$domainname_executable" ]; then
>>>>
>>
>> (or 'if command -v domainname >/dev/null 2>&1; then domainname ...')
>>
>>
>>> Yeah, this seems better for me.
>>
>> Me too. Glenn, could you send a complete patch doing this? I'll add it
>> to the kbuild tree then.
>>
>> Thanks,
>> Michal
>>
>
> Yeah, good idea with "command -v" ! :)
> ( note: `command -v` will return true if the executable is found -
> else it will return false. )
>
> mkcompile_h is changed slightly in 2.6.32. Here's my new proposed patch:
>
>
> --- scripts/mkcompile_h.orig    2010-01-26 18:59:37.000000000 +0100
> +++ scripts/mkcompile_h 2010-01-26 20:03:42.000000000 +0100
> @@ -67,9 +67,9 @@
>   echo \#define LINUX_COMPILE_BY \"`whoami`\"
>   echo \#define LINUX_COMPILE_HOST \"`hostname | $UTS_TRUNCATE`\"
>
> -  if [ -x /bin/dnsdomainname ]; then
> +  if [ `command -v dnsdomainname 2> /dev/null` ]; then
>     domain=`dnsdomainname 2> /dev/null`
> -  elif [ -x /bin/domainname ]; then
> +  elif [ `command -v domainname 2> /dev/null` ]; then
>     domain=`domainname 2> /dev/null`
>   fi
>

No, this doesn't look good.

First, you don't need to redirect stderr for 'command'.

Second, 'command' also searches in shell built-in commands, aliases,
so I prefer 'whereis -b'.

Re: [PATCH] Use full path to dnsdomainname and domainname in scripts/mkcompile_h

[Michal Marek]

On Wed, Jan 27, 2010 at 10:44:29AM +0800, Américo Wang wrote:
> On Wed, Jan 27, 2010 at 3:10 AM, Glenn Sommer <glemsom@gmail.com> wrote:
> > --- scripts/mkcompile_h.orig    2010-01-26 18:59:37.000000000 +0100
> > +++ scripts/mkcompile_h 2010-01-26 20:03:42.000000000 +0100
> > @@ -67,9 +67,9 @@
> >   echo \#define LINUX_COMPILE_BY \"`whoami`\"
> >   echo \#define LINUX_COMPILE_HOST \"`hostname | $UTS_TRUNCATE`\"
> >
> > -  if [ -x /bin/dnsdomainname ]; then
> > +  if [ `command -v dnsdomainname 2> /dev/null` ]; then
> >     domain=`dnsdomainname 2> /dev/null`
> > -  elif [ -x /bin/domainname ]; then
> > +  elif [ `command -v domainname 2> /dev/null` ]; then
> >     domain=`domainname 2> /dev/null`
> >   fi
> >
> 
> No, this doesn't look good.
> 
> First, you don't need to redirect stderr for 'command'.
> 
> Second, 'command' also searches in shell built-in commands, aliases,
> so I prefer 'whereis -b'.


Well, 'command -v domainname' returns success iff 'domainname' can be
executed (be it an external command, builtin, function, whatever), which
is exactly what we do on the next line. But, there is no need to capture
the output of 'command -v domainname' and pass it to [ ... ], just test
the return code.

... crap, now I learned that busybox doesn't support 'command' :-(
So what about simply trying 'dnsdomainname' and falling back to
domainname if it fails? Like this:


Subject: [PATCH] scripts/mkcompile_h: don't test for hardcoded paths

Don't test for /bin/{dnsdomainname,domainname}, simply try to execute
the command and check if it returned something.

Reported-by: Glenn Sommer <glemsom@gmail.com>
Signed-off-by: Michal Marek <mmarek@suse.cz>
---
 scripts/mkcompile_h |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/scripts/mkcompile_h b/scripts/mkcompile_h
index 23dbad8..50ad317 100755
--- a/scripts/mkcompile_h
+++ b/scripts/mkcompile_h
@@ -67,9 +67,8 @@ UTS_TRUNCATE="cut -b -$UTS_LEN"
   echo \#define LINUX_COMPILE_BY \"`whoami`\"
   echo \#define LINUX_COMPILE_HOST \"`hostname | $UTS_TRUNCATE`\"
 
-  if [ -x /bin/dnsdomainname ]; then
-    domain=`dnsdomainname 2> /dev/null`
-  elif [ -x /bin/domainname ]; then
+  domain=`dnsdomainname 2> /dev/null`
+  if [ -z "$domain" ]; then
     domain=`domainname 2> /dev/null`
   fi
 
-- 
1.6.5.3

Re: [PATCH] Use full path to dnsdomainname and domainname in scripts/mkcompile_h

[Américo Wang]

On Wed, Jan 27, 2010 at 4:52 PM, Michal Marek <mmarek@suse.cz> wrote:
> On Wed, Jan 27, 2010 at 10:44:29AM +0800, Américo Wang wrote:
>> On Wed, Jan 27, 2010 at 3:10 AM, Glenn Sommer <glemsom@gmail.com> wrote:
>> > --- scripts/mkcompile_h.orig    2010-01-26 18:59:37.000000000 +0100
>> > +++ scripts/mkcompile_h 2010-01-26 20:03:42.000000000 +0100
>> > @@ -67,9 +67,9 @@
>> >   echo \#define LINUX_COMPILE_BY \"`whoami`\"
>> >   echo \#define LINUX_COMPILE_HOST \"`hostname | $UTS_TRUNCATE`\"
>> >
>> > -  if [ -x /bin/dnsdomainname ]; then
>> > +  if [ `command -v dnsdomainname 2> /dev/null` ]; then
>> >     domain=`dnsdomainname 2> /dev/null`
>> > -  elif [ -x /bin/domainname ]; then
>> > +  elif [ `command -v domainname 2> /dev/null` ]; then
>> >     domain=`domainname 2> /dev/null`
>> >   fi
>> >
>>
>> No, this doesn't look good.
>>
>> First, you don't need to redirect stderr for 'command'.
>>
>> Second, 'command' also searches in shell built-in commands, aliases,
>> so I prefer 'whereis -b'.
>
>
> Well, 'command -v domainname' returns success iff 'domainname' can be
> executed (be it an external command, builtin, function, whatever), which
> is exactly what we do on the next line. But, there is no need to capture
> the output of 'command -v domainname' and pass it to [ ... ], just test
> the return code.
>
> ... crap, now I learned that busybox doesn't support 'command' :-(
> So what about simply trying 'dnsdomainname' and falling back to
> domainname if it fails? Like this:
>
>
> Subject: [PATCH] scripts/mkcompile_h: don't test for hardcoded paths
>
> Don't test for /bin/{dnsdomainname,domainname}, simply try to execute
> the command and check if it returned something.

Good!

>
> Reported-by: Glenn Sommer <glemsom@gmail.com>
> Signed-off-by: Michal Marek <mmarek@suse.cz>

Acked-by: WANG Cong <xiyou.wangcong@gmail.com>

> ---
>  scripts/mkcompile_h |    5 ++---
>  1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/mkcompile_h b/scripts/mkcompile_h
> index 23dbad8..50ad317 100755
> --- a/scripts/mkcompile_h
> +++ b/scripts/mkcompile_h
> @@ -67,9 +67,8 @@ UTS_TRUNCATE="cut -b -$UTS_LEN"
>   echo \#define LINUX_COMPILE_BY \"`whoami`\"
>   echo \#define LINUX_COMPILE_HOST \"`hostname | $UTS_TRUNCATE`\"
>
> -  if [ -x /bin/dnsdomainname ]; then
> -    domain=`dnsdomainname 2> /dev/null`
> -  elif [ -x /bin/domainname ]; then
> +  domain=`dnsdomainname 2> /dev/null`
> +  if [ -z "$domain" ]; then
>     domain=`domainname 2> /dev/null`
>   fi
>
> --
> 1.6.5.3
>
>

Re: [PATCH] Use full path to dnsdomainname and domainname in scripts/mkcompile_h

[Glenn Sommer]

2010/1/27 Michal Marek <mmarek@suse.cz>:
> On Wed, Jan 27, 2010 at 10:44:29AM +0800, Américo Wang wrote:
>> On Wed, Jan 27, 2010 at 3:10 AM, Glenn Sommer <glemsom@gmail.com> wrote:
>> > --- scripts/mkcompile_h.orig    2010-01-26 18:59:37.000000000 +0100
>> > +++ scripts/mkcompile_h 2010-01-26 20:03:42.000000000 +0100
>> > @@ -67,9 +67,9 @@
>> >   echo \#define LINUX_COMPILE_BY \"`whoami`\"
>> >   echo \#define LINUX_COMPILE_HOST \"`hostname | $UTS_TRUNCATE`\"
>> >
>> > -  if [ -x /bin/dnsdomainname ]; then
>> > +  if [ `command -v dnsdomainname 2> /dev/null` ]; then
>> >     domain=`dnsdomainname 2> /dev/null`
>> > -  elif [ -x /bin/domainname ]; then
>> > +  elif [ `command -v domainname 2> /dev/null` ]; then
>> >     domain=`domainname 2> /dev/null`
>> >   fi
>> >
>>
>> No, this doesn't look good.
>>
>> First, you don't need to redirect stderr for 'command'.
>>
>> Second, 'command' also searches in shell built-in commands, aliases,
>> so I prefer 'whereis -b'.
>
>
> Well, 'command -v domainname' returns success iff 'domainname' can be
> executed (be it an external command, builtin, function, whatever), which
> is exactly what we do on the next line. But, there is no need to capture
> the output of 'command -v domainname' and pass it to [ ... ], just test
> the return code.

> ... crap, now I learned that busybox doesn't support 'command' :-(
> So what about simply trying 'dnsdomainname' and falling back to
> domainname if it fails? Like this:


Ohh, I didn't know that! We DO need to be compatible with busybox! :/


>
> Subject: [PATCH] scripts/mkcompile_h: don't test for hardcoded paths
>
> Don't test for /bin/{dnsdomainname,domainname}, simply try to execute
> the command and check if it returned something.
>
> Reported-by: Glenn Sommer <glemsom@gmail.com>
> Signed-off-by: Michal Marek <mmarek@suse.cz>
> ---
>  scripts/mkcompile_h |    5 ++---
>  1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/mkcompile_h b/scripts/mkcompile_h
> index 23dbad8..50ad317 100755
> --- a/scripts/mkcompile_h
> +++ b/scripts/mkcompile_h
> @@ -67,9 +67,8 @@ UTS_TRUNCATE="cut -b -$UTS_LEN"
>   echo \#define LINUX_COMPILE_BY \"`whoami`\"
>   echo \#define LINUX_COMPILE_HOST \"`hostname | $UTS_TRUNCATE`\"
>
> -  if [ -x /bin/dnsdomainname ]; then
> -    domain=`dnsdomainname 2> /dev/null`
> -  elif [ -x /bin/domainname ]; then
> +  domain=`dnsdomainname 2> /dev/null`
> +  if [ -z "$domain" ]; then
>     domain=`domainname 2> /dev/null`
>   fi
>
> --
> 1.6.5.3
>
>

I tested above patch, and it seems to work fine.
Though, by looking a bit closer at the source - I found we actually
NEVER need to use 2> /dev/null.
We capture the output using ") > .tmpcompile", meaning we only capture
stdout _NOT_ stderr.
One could argue that we should remove the redirection of stderr for
debugging purposes. (In really rare cases where both dnsdomianname and
domainname are missing)
Though, I do NOT see the need for that!

I'm completely satisfied with the above patch! It does the job I
initially requested - which was to either use a complete path at all
times, or never use a complete path. :)

Re: [PATCH] Use full path to dnsdomainname and domainname in scripts/mkcompile_h

[Michal Marek]

On 27.1.2010 11:14, Glenn Sommer wrote:
> 2010/1/27 Michal Marek <mmarek@suse.cz>:
>> On Wed, Jan 27, 2010 at 10:44:29AM +0800, Américo Wang wrote:
>>> On Wed, Jan 27, 2010 at 3:10 AM, Glenn Sommer <glemsom@gmail.com> wrote:
>>>> --- scripts/mkcompile_h.orig    2010-01-26 18:59:37.000000000 +0100
>>>> +++ scripts/mkcompile_h 2010-01-26 20:03:42.000000000 +0100
>>>> @@ -67,9 +67,9 @@
>>>>   echo \#define LINUX_COMPILE_BY \"`whoami`\"
>>>>   echo \#define LINUX_COMPILE_HOST \"`hostname | $UTS_TRUNCATE`\"
>>>>
>>>> -  if [ -x /bin/dnsdomainname ]; then
>>>> +  if [ `command -v dnsdomainname 2> /dev/null` ]; then
>>>>     domain=`dnsdomainname 2> /dev/null`
>>>> -  elif [ -x /bin/domainname ]; then
>>>> +  elif [ `command -v domainname 2> /dev/null` ]; then
>>>>     domain=`domainname 2> /dev/null`
>>>>   fi
>>>>
>>>
>>> No, this doesn't look good.
>>>
>>> First, you don't need to redirect stderr for 'command'.
>>>
>>> Second, 'command' also searches in shell built-in commands, aliases,
>>> so I prefer 'whereis -b'.
>>
>>
>> Well, 'command -v domainname' returns success iff 'domainname' can be
>> executed (be it an external command, builtin, function, whatever), which
>> is exactly what we do on the next line. But, there is no need to capture
>> the output of 'command -v domainname' and pass it to [ ... ], just test
>> the return code.
> 
>> ... crap, now I learned that busybox doesn't support 'command' :-(
>> So what about simply trying 'dnsdomainname' and falling back to
>> domainname if it fails? Like this:
> 
> 
> Ohh, I didn't know that! We DO need to be compatible with busybox! :/
> 
> 
>>
>> Subject: [PATCH] scripts/mkcompile_h: don't test for hardcoded paths
>>
>> Don't test for /bin/{dnsdomainname,domainname}, simply try to execute
>> the command and check if it returned something.
>>
>> Reported-by: Glenn Sommer <glemsom@gmail.com>
>> Signed-off-by: Michal Marek <mmarek@suse.cz>
>> ---
>>  scripts/mkcompile_h |    5 ++---
>>  1 files changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/scripts/mkcompile_h b/scripts/mkcompile_h
>> index 23dbad8..50ad317 100755
>> --- a/scripts/mkcompile_h
>> +++ b/scripts/mkcompile_h
>> @@ -67,9 +67,8 @@ UTS_TRUNCATE="cut -b -$UTS_LEN"
>>   echo \#define LINUX_COMPILE_BY \"`whoami`\"
>>   echo \#define LINUX_COMPILE_HOST \"`hostname | $UTS_TRUNCATE`\"
>>
>> -  if [ -x /bin/dnsdomainname ]; then
>> -    domain=`dnsdomainname 2> /dev/null`
>> -  elif [ -x /bin/domainname ]; then
>> +  domain=`dnsdomainname 2> /dev/null`
>> +  if [ -z "$domain" ]; then
>>     domain=`domainname 2> /dev/null`
>>   fi
>>
>> --
>> 1.6.5.3
>>
>>
> 
> I tested above patch, and it seems to work fine.

Thanks! I added a Tested-by: Glenn Sommer <glemsom@gmail.com> line to
the patch and pushed to the kbuild tree.


> Though, by looking a bit closer at the source - I found we actually
> NEVER need to use 2> /dev/null.

The redirecion was added by

commit 9c3049c02c6142e166c9472237f1f60d86153682
Author: Felipe Contreras <felipe.contreras@gmail.com>
Date:   Thu Sep 17 00:38:39 2009 +0300

    kbuild: fix warning when domainname is not available


and it suits me well because it hides the potential "command not found
message" :).

Michal