[PATCH] Use full path to dnsdomainname and domainname in scripts/mkcompile_h
Glenn Sommer
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 <gle...@gmail.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Américo Wang
> 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 <gle...@gmail.com>
Makes sense, but is that possible we have 'domainname' installed in two
different directories?
Glenn Sommer
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
Américo Wang
Yeah, this seems better for me.
Thanks!
Michal Marek
(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
Glenn Sommer
> On 26.1.2010 04:55, Américo Wang wrote:
>> On Wed, Jan 20, 2010 at 11:06 PM, Glenn Sommer <gle...@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 <gle...@gmail.com>
Américo Wang
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'.
Michal Marek
> On Wed, Jan 27, 2010 at 3:10 AM, Glenn Sommer <gle...@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 <gle...@gmail.com>
Signed-off-by: Michal Marek <mma...@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
Américo Wang
Good!
>
> Reported-by: Glenn Sommer <gle...@gmail.com>
> Signed-off-by: Michal Marek <mma...@suse.cz>
Acked-by: WANG Cong <xiyou.w...@gmail.com>
Glenn Sommer
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 <gle...@gmail.com>
> Signed-off-by: Michal Marek <mma...@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. :)
Michal Marek
Thanks! I added a Tested-by: Glenn Sommer <gle...@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.c...@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