[PATCH 1/3] CLI: add talloc leak report, controlled by an environment variable.

[Austin Clements]

Quoth david at tethera.net on Dec 16 at 11:24 pm:
> From: David Bremner <bremner at debian.org>
> 
> The argument handling in notmuch.c seems due for an overhaul, but
> until then use an environment variable to specify a location to write
> the talloc leak report to.  This is only enabled for the (interesting)
> case where some notmuch subcommand is invoked.
> ---
>  notmuch.c |   16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/notmuch.c b/notmuch.c
> index 9516dfb..fb49c5a 100644
> --- a/notmuch.c
> +++ b/notmuch.c
> @@ -322,8 +322,20 @@ main (int argc, char *argv[])
>      for (i = 0; i < ARRAY_SIZE (commands); i++) {
>  	command = &commands[i];
>  
> -	if (strcmp (argv[1], command->name) == 0)
> -	    return (command->function) (local, argc - 1, &argv[1]);
> +	if (strcmp (argv[1], command->name) == 0) {
> +	    int ret;
> +	    char *talloc_report;
> +
> +	    ret = (command->function) (local, argc - 1, &argv[1]);
> +
> +	    talloc_report=getenv ("NOTMUCH_TALLOC_REPORT");

Missing spaces around =.

I think hacking this in as an environment variable is fine, but maybe
there should be a comment here saying that it would be better to
follow Samba's talloc command-line argument conventions?

> +	    if (talloc_report && strcmp(talloc_report, "") != 0) {

Missing space before paren.

> +		FILE *report = fopen (talloc_report, "w");
> +		talloc_report_full (NULL, report);

Maybe I'm missing something here, but don't you have to call
talloc_enable_leak_report_full before the first use of talloc to get a
complete leak report?

> +	    }
> +
> +	    return ret;
> +	}
>      }
>  
>      fprintf (stderr, "Error: Unknown command '%s' (see \"notmuch help\")\n",