Messages in this thread |  | | | Date | Wed, 27 Jan 2010 14:15:24 +0100 | | From | Michal Marek <> | | Subject | Re: [PATCH] Use full path to dnsdomainname and domainname in scripts/mkcompile_h |
| |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |