[PATCH v3 01/16] add util/search-path.{c, h} to test for executables in $PATH

[David Bremner]

Daniel Kahn Gillmor <dkg at fifthhorseman.net> writes:

> +notmuch_bool_t
> +test_for_executable(const char* exename)
> +{
> +    char *c = NULL, *save = NULL, *tok;
> +    size_t n;
> +    int dfd = -1;

c, dfd, and n could be more meaningful as variable names.

% uncrustify --config devel/uncrustify.cfg --replace util/search-path.c util/search-path.h

yields quite a few whitespace changes (diff attached)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: wschanges.diff
Type: text/x-diff
Size: 2191 bytes
Desc: not available
URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20160209/840c7044/attachment.diff>
-------------- next part --------------

> +    notmuch_bool_t ret = FALSE;
> +
> +    if (strchr(exename, '/')) {
> +	if (0 == access(exename, X_OK))
> +	    return TRUE;
> +	else
> +	    return FALSE;
> +    }
> +    
> +    c = getenv("PATH");
> +    if (c)
> +	c = talloc_strdup(NULL, c);

Is there some advantage to using the talloc_ functions here?

> +    else {

Is n needed outside this block? if not, it could be declared here (and
then I don't care about the single letter name).

> +	n = confstr(_CS_PATH, NULL, 0);

according to a glance at the man page, this might return 0 if there is
no value for _CS_PATH set?


> +
> +    tok = strtok_r(c, ":", &save);
> +    while (tok) {
same comment about block local declaration of dfd

> +	dfd = open(tok, O_DIRECTORY | O_RDONLY);

> +	tok = strtok_r(NULL, ":", &save);

not sure if it helps, but there is also strtok_len in libutil

> +done:

as a real nitpick, we have always (?) used DONE for a label.