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

2012-12-20 Thread Austin Clements
Quoth david at tethera.net on Dec 16 at 11:24 pm:
> From: David Bremner 
> 
> 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 = [i];
>  
> - if (strcmp (argv[1], command->name) == 0)
> - return (command->function) (local, argc - 1, [1]);
> + if (strcmp (argv[1], command->name) == 0) {
> + int ret;
> + char *talloc_report;
> +
> + ret = (command->function) (local, argc - 1, [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",


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

2012-12-20 Thread Tomi Ollila
On Mon, Dec 17 2012, david at tethera.net wrote:

> From: David Bremner 
>
> 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.
> ---

I'd still suggest to have similar command-line interface option(s) as
samba4 use: --leak-report and/or --leak-report-full

(search --leak-r in talloc(3) namual page)

Tomi

>  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 = [i];
>  
> - if (strcmp (argv[1], command->name) == 0)
> - return (command->function) (local, argc - 1, [1]);
> + if (strcmp (argv[1], command->name) == 0) {
> + int ret;
> + char *talloc_report;
> +
> + ret = (command->function) (local, argc - 1, [1]);
> +
> + talloc_report=getenv ("NOTMUCH_TALLOC_REPORT");
> + if (talloc_report && strcmp(talloc_report, "") != 0) {
> + FILE *report = fopen (talloc_report, "w");
> + talloc_report_full (NULL, report);
> + }
> +
> + return ret;
> + }
>  }
>  
>  fprintf (stderr, "Error: Unknown command '%s' (see \"notmuch help\")\n",
> -- 
> 1.7.10.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


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

2012-12-20 Thread David Bremner
Tomi Ollila  writes:

> On Mon, Dec 17 2012, david at tethera.net wrote:
>
>> From: David Bremner 
>>
>> 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.
>> ---
>
> I'd still suggest to have similar command-line interface option(s) as
> samba4 use: --leak-report and/or --leak-report-full
>
> (search --leak-r in talloc(3) namual page)
>

Sure sounds fine, when someone (TM) redoes the top level argument
handling. I think it would make sense to ditch the alias handling
completely; both current aliases are deprecated for a long time.

d


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

2012-12-20 Thread Tomi Ollila
On Mon, Dec 17 2012, da...@tethera.net wrote:

 From: David Bremner brem...@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.
 ---

I'd still suggest to have similar command-line interface option(s) as
samba4 use: --leak-report and/or --leak-report-full

(search --leak-r in talloc(3) namual page)

Tomi

  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);
 + if (talloc_report  strcmp(talloc_report, ) != 0) {
 + FILE *report = fopen (talloc_report, w);
 + talloc_report_full (NULL, report);
 + }
 +
 + return ret;
 + }
  }
  
  fprintf (stderr, Error: Unknown command '%s' (see \notmuch help\)\n,
 -- 
 1.7.10.4

 ___
 notmuch mailing list
 notmuch@notmuchmail.org
 http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


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

2012-12-20 Thread David Bremner
Tomi Ollila tomi.oll...@iki.fi writes:

 On Mon, Dec 17 2012, da...@tethera.net wrote:

 From: David Bremner brem...@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.
 ---

 I'd still suggest to have similar command-line interface option(s) as
 samba4 use: --leak-report and/or --leak-report-full

 (search --leak-r in talloc(3) namual page)


Sure sounds fine, when someone (TM) redoes the top level argument
handling. I think it would make sense to ditch the alias handling
completely; both current aliases are deprecated for a long time.

d
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


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

2012-12-20 Thread Austin Clements
Quoth da...@tethera.net on Dec 16 at 11:24 pm:
 From: David Bremner brem...@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,
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


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

2012-12-16 Thread da...@tethera.net
From: David Bremner 

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 = [i];

-   if (strcmp (argv[1], command->name) == 0)
-   return (command->function) (local, argc - 1, [1]);
+   if (strcmp (argv[1], command->name) == 0) {
+   int ret;
+   char *talloc_report;
+
+   ret = (command->function) (local, argc - 1, [1]);
+
+   talloc_report=getenv ("NOTMUCH_TALLOC_REPORT");
+   if (talloc_report && strcmp(talloc_report, "") != 0) {
+   FILE *report = fopen (talloc_report, "w");
+   talloc_report_full (NULL, report);
+   }
+
+   return ret;
+   }
 }

 fprintf (stderr, "Error: Unknown command '%s' (see \"notmuch help\")\n",
-- 
1.7.10.4



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

2012-12-16 Thread david
From: David Bremner brem...@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);
+   if (talloc_report  strcmp(talloc_report, ) != 0) {
+   FILE *report = fopen (talloc_report, w);
+   talloc_report_full (NULL, report);
+   }
+
+   return ret;
+   }
 }
 
 fprintf (stderr, Error: Unknown command '%s' (see \notmuch help\)\n,
-- 
1.7.10.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch