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