Re: [PATCH 6/7] include $PATH in generating list of commands for "help -a"

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi,

On Sun, 28 Oct 2007, Scott R Parish wrote:

> diff --git a/help.c b/help.c
> index 34ac5db..07cf67a 100644
> --- a/help.c
> +++ b/help.c
> @@ -37,24 +37,28 @@ static inline void mput_char(char c, unsigned int num)
>  		putchar(c);
>  }
>  
> -static struct cmdname {
> -	size_t len;
> -	char name[1];
> -} **cmdname;
> -static int cmdname_alloc, cmdname_cnt;
> -
> -static void add_cmdname(const char *name, int len)
> +static struct cmdnames {
> +	int alloc;
> +	int cnt;
> +	struct cmdname {
> +		size_t len;
> +		char name[1];
> +	} **names;
> +} main_cmds, other_cmds;
> +
> +static void add_cmdname(struct cmdnames *cmds, const char *name, int len)
>  {
>  	struct cmdname *ent;
> -	if (cmdname_alloc <= cmdname_cnt) {
> -		cmdname_alloc = cmdname_alloc + 200;
> -		cmdname = xrealloc(cmdname, cmdname_alloc * sizeof(*cmdname));
> +	if (cmds->alloc <= cmds->cnt) {
> +		cmds->alloc = cmds->alloc + 200;
> +		cmds->names = xrealloc(cmds->names,
> +				       cmds->alloc * sizeof(*cmds->names));

Looks like a candidate for ALLOC_GROW() ...

> @@ -64,7 +68,44 @@ static int cmdname_compare(const void *a_, const void *b_)
>  	return strcmp(a->name, b->name);
>  }
>  
> -static void pretty_print_string_list(struct cmdname **cmdname, int longest)
> +static void uniq(struct cmdnames *cmds)
> +{
> +	int i, j;
> +
> +	if (!cmds->cnt)
> +		return;
> +
> +	for (i = j = 1; i < cmds->cnt; i++) {
> +		if (strcmp(cmds->names[i]->name, cmds->names[i-1]->name)) {
> +			cmds->names[j++] = cmds->names[i];
> +		}
> +	}

Losing the curly brackets would make this look much nicer.

> +
> +	cmds->cnt = j;
> +}
> +
> +static void subtract_cmds(struct cmdnames *a, struct cmdnames *b) {

Maybe "exclude_cmds()", and choose more suggestive names for the 
parameters?

> -	DIR *dir = opendir(exec_path);
> +	DIR *dirp = opendir(dir);

I am not sure that a rename from "dir" to "dirp" is needed here.  It 
distracts a little from the real content of your patch.

Thanks,
Dscho