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

Michal Marek
2010-01-27T13:15:27Z

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