All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] whereis: search in path variable
@ 2011-07-20  4:46 Davidlohr Bueso
  2011-07-21 16:51 ` Karel Zak
  0 siblings, 1 reply; 3+ messages in thread
From: Davidlohr Bueso @ 2011-07-20  4:46 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux

From: Davidlohr Bueso <dave@gnu.org>
Date: Wed, 20 Jul 2011 00:39:10 -0400

Currently this tool only uses the hardcoded paths for looking up strings for binaries, man pages and source code,
adding those directories found in $PATH makes a nice little enhancement.

dave@offbook:~/projects/util-linux/misc-utils$ export PATH=$PATH:/home/dave/whereis-test
dave@offbook:~/projects/util-linux/misc-utils$ ./whereis stdlib
stdlib: /home/dave/whereis-test/stdlib.h /usr/include/stdlib.h
dave@offbook:~/projects/util-linux/misc-utils$ whereis-old stdlib
stdlib: /usr/include/stdlib.h

This feature was also discussed previously here (http://www.spinics.net/lists/util-linux-ng/msg03429.html)

Signed-off-by: Davidlohr Bueso <dave@gnu.org>
---
 misc-utils/whereis.c |   53 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 53 insertions(+), 0 deletions(-)

diff --git a/misc-utils/whereis.c b/misc-utils/whereis.c
index 4f841f9..ec75057 100644
--- a/misc-utils/whereis.c
+++ b/misc-utils/whereis.c
@@ -45,6 +45,8 @@
 #include <stdlib.h>
 #include <string.h>
 #include <ctype.h>
+
+#include "xalloc.h"
 #include "nls.h"
 #include "c.h"
 
@@ -58,6 +60,7 @@ void findv(char **, int, char *);
 void find(char **, char *);
 void findin(char *, char *);
 int itsit(char *, char *);
+void fillpath(void);
 
 static char *bindirs[] = {
 	"/bin",
@@ -144,6 +147,7 @@ int	Bcnt;
 char	**Mflag;
 int	Mcnt;
 char	uflag;
+char    **dirp, **pathdir = NULL;
 
 static void __attribute__ ((__noreturn__)) usage(FILE * out)
 {
@@ -172,6 +176,8 @@ static void __attribute__ ((__noreturn__)) usage(FILE * out)
 int
 main(int argc, char **argv)
 {
+	int i;
+
 	setlocale(LC_ALL, "");
 	bindtextdomain(PACKAGE, LOCALEDIR);
 	textdomain(PACKAGE);
@@ -179,6 +185,9 @@ main(int argc, char **argv)
 	argc--, argv++;
 	if (argc == 0)
 		usage(stderr);
+	
+	fillpath();
+	dirp = pathdir;
 
 	do
 		if (argv[0][0] == '-') {
@@ -232,9 +241,51 @@ main(int argc, char **argv)
 		} else
 			lookup(*argv++);
 	while (--argc > 0);
+
+	free(pathdir);
+
 	return EXIT_SUCCESS;
 }
 
+int inpath(const char *str)
+{
+	int i;
+
+	for (i = 0; i < sizeof(bindirs)/sizeof(char *) - 1 ; i++)
+		if (!strcmp(bindirs[i], str))
+			return 1;
+	
+	for (i = 0; i < sizeof(mandirs)/sizeof(char *) - 1; i++)
+		if (!strcmp(mandirs[i], str))
+			return 1;
+		
+	for (i = 0; i < sizeof(srcdirs)/sizeof(char *) - 1; i++)
+		if (!strcmp(srcdirs[i], str))
+			return 1;
+	
+	return 0;
+}
+
+void fillpath(void)
+{
+	char *key=NULL, *tmp=NULL, *tok=NULL, *path = getenv("PATH");
+	int i = 0;
+
+	if (!path)
+		return;
+
+	for (tmp = path; ;tmp = NULL) {
+		tok = strtok_r(tmp, ":", &key);
+		if (!tok)
+			break;
+		if (inpath(tok)) /* make sure we don't repeat the search path */
+			continue;
+
+		pathdir = xrealloc(pathdir, (i + 1) * sizeof(char *));
+		pathdir[i++] = tok;
+	}
+}
+
 void
 getlist(int *argcp, char ***argvp, char ***flagp, int *cntp)
 {
@@ -356,6 +407,8 @@ find(char **dirs, char *cp)
 {
 	while (*dirs)
 		findin(*dirs++, cp);
+	while(*dirp)
+		findin(*dirp++, cp);
 }
 
 void
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] whereis: search in path variable
  2011-07-20  4:46 [PATCH] whereis: search in path variable Davidlohr Bueso
@ 2011-07-21 16:51 ` Karel Zak
  2011-07-24 23:01   ` Davidlohr Bueso
  0 siblings, 1 reply; 3+ messages in thread
From: Karel Zak @ 2011-07-21 16:51 UTC (permalink / raw)
  To: Davidlohr Bueso; +Cc: util-linux

On Wed, Jul 20, 2011 at 12:46:25AM -0400, Davidlohr Bueso wrote:
> From: Davidlohr Bueso <dave@gnu.org>
> Date: Wed, 20 Jul 2011 00:39:10 -0400
> 
> Currently this tool only uses the hardcoded paths for looking up
> strings for binaries, man pages and source code, adding those
> directories found in $PATH makes a nice little enhancement.

 Finally someone who is not lazy to implement it correctly :-)

> +char    **dirp, **pathdir = NULL;

 It's unnecessary to initialize global variables to 0 or NULL.

>  main(int argc, char **argv)
>  {
> +	int i;

 variable not used

>  	setlocale(LC_ALL, "");
>  	bindtextdomain(PACKAGE, LOCALEDIR);
>  	textdomain(PACKAGE);
> @@ -179,6 +185,9 @@ main(int argc, char **argv)
>  	argc--, argv++;
>  	if (argc == 0)
>  		usage(stderr);
> +	
> +	fillpath();
> +	dirp = pathdir;

 it would be better to hide 'dirp = pathdir' to fillpath().

>  	do
>  		if (argv[0][0] == '-') {
> @@ -232,9 +241,51 @@ main(int argc, char **argv)
>  		} else
>  			lookup(*argv++);
>  	while (--argc > 0);
> +
> +	free(pathdir);

 add freepath()
>  
> +int inpath(const char *str)
> +{
> +	int i;
> +
> +	for (i = 0; i < sizeof(bindirs)/sizeof(char *) - 1 ; i++)
                    ^^^^^^^^^^^^^^^^^^^
 We have ARRAY_SIZE() in c.h, or you can use the fact that arrays are
 terminated by zero.

> +		if (!strcmp(bindirs[i], str))
> +			return 1;
> +	
> +	for (i = 0; i < sizeof(mandirs)/sizeof(char *) - 1; i++)
> +		if (!strcmp(mandirs[i], str))
> +			return 1;
> +		
> +	for (i = 0; i < sizeof(srcdirs)/sizeof(char *) - 1; i++)
> +		if (!strcmp(srcdirs[i], str))
> +			return 1;
> +	
> +	return 0;
> +}
> +
> +void fillpath(void)
> +{
> +	char *key=NULL, *tmp=NULL, *tok=NULL, *path = getenv("PATH");
> +	int i = 0;
> +
> +	if (!path)
> +		return;
> +
> +	for (tmp = path; ;tmp = NULL) {
> +		tok = strtok_r(tmp, ":", &key);

 man getenv, you should not modify the result from getenv().

> +		if (!tok)
> +			break;
> +		if (inpath(tok)) /* make sure we don't repeat the search path */
> +			continue;
> +
> +		pathdir = xrealloc(pathdir, (i + 1) * sizeof(char *));
> +		pathdir[i++] = tok;
> +	}

 here is bug, see below to gdb backtrace...

> +
>  void
>  getlist(int *argcp, char ***argvp, char ***flagp, int *cntp)
>  {
> @@ -356,6 +407,8 @@ find(char **dirs, char *cp)
>  {
>  	while (*dirs)
>  		findin(*dirs++, cp);
> +	while(*dirp)
          ^^^^^^
> +		findin(*dirp++, cp);

 ... this code expects that the array is terminated by zero.

Note that find() is called always for all dir lists.

Maybe it would be better to add lookpathenv() and call it from
print_again(). You can also add -p options to control this behavior.
See how {s,b,m}flags work.

BTW, the whole whereis code is horrible, for example find() is completely
unnecessary if there is also findv() and all lists of the directories are
static. It should be possible to use

  findv(ary, ARRAY_SIZE(ary), str);

everywhere instead of find(ary, str);

    Karel

Starting program: /home/projects/util-linux/util-linux/misc-utils/whereis lsblk

Program received signal SIGSEGV, Segmentation fault.
0x0000003cbae7edfa in __strchr_sse2 () from /lib64/libc.so.6
Missing separate debuginfos, use: debuginfo-install glibc-2.14-4.x86_64
(gdb) bt
#0  0x0000003cbae7edfa in __strchr_sse2 () from /lib64/libc.so.6
#1  0x00000000004013c8 in findin (dir=0x1ff41 <Address 0x1ff41 out of bounds>, 
    cp=0x7fffffffe477 "lsblk") at whereis.c:434
#2  0x0000000000401623 in find (dirs=<optimized out>, 
    cp=0x7fffffffe477 "lsblk") at whereis.c:421
#3  0x00000000004017a8 in print_again (cp=0x7fffffffe477 "lsblk")
    at whereis.c:327
#4  0x0000000000401870 in lookup (cp=0x7fffffffe477 "lsblk") at whereis.c:374
#5  0x0000000000400c9b in main (argc=1, argv=0x7fffffffe148) at whereis.c:242

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] whereis: search in path variable
  2011-07-21 16:51 ` Karel Zak
@ 2011-07-24 23:01   ` Davidlohr Bueso
  0 siblings, 0 replies; 3+ messages in thread
From: Davidlohr Bueso @ 2011-07-24 23:01 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux

On Thu, 2011-07-21 at 18:51 +0200, Karel Zak wrote:
> On Wed, Jul 20, 2011 at 12:46:25AM -0400, Davidlohr Bueso wrote:
> > From: Davidlohr Bueso <dave@gnu.org>
> > Date: Wed, 20 Jul 2011 00:39:10 -0400
> > 
> > Currently this tool only uses the hardcoded paths for looking up
> > strings for binaries, man pages and source code, adding those
> > directories found in $PATH makes a nice little enhancement.
> 
>  Finally someone who is not lazy to implement it correctly :-)
...
>  man getenv, you should not modify the result from getenv().

ouch, sorry!

> 
> > +		if (!tok)
> > +			break;
> > +		if (inpath(tok)) /* make sure we don't repeat the search path */
> > +			continue;
> > +
> > +		pathdir = xrealloc(pathdir, (i + 1) * sizeof(char *));
> > +		pathdir[i++] = tok;
> > +	}
> 
>  here is bug, see below to gdb backtrace...
> 
> > +
> >  void
> >  getlist(int *argcp, char ***argvp, char ***flagp, int *cntp)
> >  {
> > @@ -356,6 +407,8 @@ find(char **dirs, char *cp)
> >  {
> >  	while (*dirs)
> >  		findin(*dirs++, cp);
> > +	while(*dirp)
>           ^^^^^^
> > +		findin(*dirp++, cp);
> 
>  ... this code expects that the array is terminated by zero.
> 
> Note that find() is called always for all dir lists.
> 
> Maybe it would be better to add lookpathenv() and call it from
> print_again(). You can also add -p options to control this behavior.
> See how {s,b,m}flags work.

That was my initial design, however since src, bin and man are _types_
of files, it seems that adding a similar behavior for search _paths_ is
like mixing apples and pears.

Something I am thinking of, but won't do it quite yet, is to simply
rewrite whereis and, instead of hardcoding paths, recursively search
from / and use stat + heuristics to differentiate the different files.

> 
> BTW, the whole whereis code is horrible, 

Like most BSD '80s code is. I was hoping you weren't going to ask for
cleanups :)

> for example find() is completely
> unnecessary if there is also findv() and all lists of the directories are
> static. It should be possible to use
> 
>   findv(ary, ARRAY_SIZE(ary), str);
> 
> everywhere instead of find(ary, str);
> 
>     Karel
> 
> Starting program: /home/projects/util-linux/util-linux/misc-utils/whereis lsblk
> 
> Program received signal SIGSEGV, Segmentation fault.

That'll teach me not to code half asleep. I'm sending you some cleanup
patches and then I'll add these fixes to the program.

> 0x0000003cbae7edfa in __strchr_sse2 () from /lib64/libc.so.6
> Missing separate debuginfos, use: debuginfo-install glibc-2.14-4.x86_64
> (gdb) bt
> #0  0x0000003cbae7edfa in __strchr_sse2 () from /lib64/libc.so.6
> #1  0x00000000004013c8 in findin (dir=0x1ff41 <Address 0x1ff41 out of bounds>, 
>     cp=0x7fffffffe477 "lsblk") at whereis.c:434
> #2  0x0000000000401623 in find (dirs=<optimized out>, 
>     cp=0x7fffffffe477 "lsblk") at whereis.c:421
> #3  0x00000000004017a8 in print_again (cp=0x7fffffffe477 "lsblk")
>     at whereis.c:327
> #4  0x0000000000401870 in lookup (cp=0x7fffffffe477 "lsblk") at whereis.c:374
> #5  0x0000000000400c9b in main (argc=1, argv=0x7fffffffe148) at whereis.c:242

- Davidlohr

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2011-07-24 23:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-20  4:46 [PATCH] whereis: search in path variable Davidlohr Bueso
2011-07-21 16:51 ` Karel Zak
2011-07-24 23:01   ` Davidlohr Bueso

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.