[patch] should crunchgen use basename(3) instead of homebrew code?

[Jordan K. Hubbard]

Wow.  A couple of questions.

1. Is it really worth messing with code as old as crunchgen(1) at all?  Seriously.  What's there isn't exactly "broken", and it seems like needlessly walking into a vampire's crypt without so much as a stake or clove of garlic on your person.  Only something bad can happen.

2. basename(3) also strips trailing '/' characters, which the code you are replacing does not.  Does this actually matter?  Hell if I know, I haven't looked at the inputs to that code in ages and I have enough common sense to stay out of musty crypts, so I'm not going to look, either. :)

- Jordan

On Jan 1, 2011, at 11:37 PM, Eitan Adler wrote:

> I'm not sure about this patch because of the behavior change of
> -    slash = strrchr(path, '/');
> -    basename = slash? slash+1 : path;
> +    bn = basename(slash);
> 
> CCed to the last few people to touch the relevant code (via svn praise)
> 
> Index: crunched_main.c
> ===================================================================
> --- crunched_main.c	(revision 216884)
> +++ crunched_main.c	(working copy)
> @@ -33,6 +33,7 @@
>  *	or calls one of them based on argv[1].   This allows the testing of
>  *	the crunched binary without creating all the links.
>  */
> +#include <libgen.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> @@ -47,22 +48,21 @@
> 
> int main(int argc, char **argv, char **envp)
> {
> -    char *slash, *basename;
> +    char *slash, *bn;
>     struct stub *ep;
> 
>     if(argv[0] == NULL || *argv[0] == '\0')
> 	crunched_usage();
> 
> -    slash = strrchr(argv[0], '/');
> -    basename = slash? slash+1 : argv[0];
> +    bn = basename(argv[0]);
> 
>     for(ep=entry_points; ep->name != NULL; ep++)
> -	if(!strcmp(basename, ep->name)) break;
> +	if(!strcmp(bn, ep->name)) break;
> 
>     if(ep->name)
> 	return ep->f(argc, argv, envp);
>     else {
> -	fprintf(stderr, "%s: %s not compiled in\n", EXECNAME, basename);
> +	fprintf(stderr, "%s: %s not compiled in\n", EXECNAME, bn);
> 	crunched_usage();
>     }
> }
> @@ -70,14 +70,13 @@
> 
> int crunched_here(char *path)
> {
> -    char *slash, *basename;
> +    char *slash, *bn;
>     struct stub *ep;
> 
> -    slash = strrchr(path, '/');
> -    basename = slash? slash+1 : path;
> +    bn = basename(slash);
> 
>     for(ep=entry_points; ep->name != NULL; ep++)
> -	if(!strcmp(basename, ep->name))
> +	if(!strcmp(bn, ep->name))
> 	    return 1;
>     return 0;
> }
> 
> 
> -- 
> Eitan Adler