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

[Michal Marek <mmarek@suse.cz>]

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