[PATCH 5/7] CLI: add --leak-report top level option

[Tomi Ollila]

On Sun, Jan 20 2013, Jameson Graef Rollins  
wrote:

> On Sat, Jan 19 2013, david at tethera.net wrote:
>> This roughly mimics the samba4 argument. The presence of the command
>> line argument overrides any value of NOTMUCH_TALLOC_REPORT in the
>> environment.
>> ---
>>  man/man1/notmuch.1 |8 
>>  notmuch.c  |   18 +++---
>>  2 files changed, 15 insertions(+), 11 deletions(-)
>>
>> diff --git a/man/man1/notmuch.1 b/man/man1/notmuch.1
>> index 6bf9b2e..5c58c41 100644
>> --- a/man/man1/notmuch.1
>> +++ b/man/man1/notmuch.1
>> @@ -70,6 +70,14 @@ Print a synopsis of available commands and exit.
>>  Print the installed version of notmuch, and exit.
>>  .RE
>>  
>> +.RS 4
>> +.TP 4
>> +.BI \-\-leak-report= path
>> +
>> +Write a detailed report of all memory allocated via talloc to
>> +.I path
>> +.RE
>
> Do we really need a command line option for this?  Why isn't the env var
> sufficient?  This just seems to me like it clutters the interface, for
> an option that is purely for debugging and should rarely if ever be used
> by most users.

Jameson does have a point. Now that we already have that environment
variable and it can be used in shipped notmuch 0.15 it is perhaps
simplest just to stick with that.

My thoughts after brief first visit to the patche series has so far
being either make the command line usage 1:1 compatible with samba
or use option like --leak-report-output=..., --leak-report-file=... or
--leak-report-to=... (and attempt to deprecate the env var...)

That said, I withdraw my previous suggestion of the command line option...

The other changes in this patch series looks initially good -- and changes
that drop deprecated features should be get in as early after last release
as possible.

> jamie.


Tomi

[PATCH 5/7] CLI: add --leak-report top level option

[Jameson Graef Rollins]

On Sat, Jan 19 2013, david at tethera.net wrote:
> This roughly mimics the samba4 argument. The presence of the command
> line argument overrides any value of NOTMUCH_TALLOC_REPORT in the
> environment.
> ---
>  man/man1/notmuch.1 |8 
>  notmuch.c  |   18 +++---
>  2 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/man/man1/notmuch.1 b/man/man1/notmuch.1
> index 6bf9b2e..5c58c41 100644
> --- a/man/man1/notmuch.1
> +++ b/man/man1/notmuch.1
> @@ -70,6 +70,14 @@ Print a synopsis of available commands and exit.
>  Print the installed version of notmuch, and exit.
>  .RE
>  
> +.RS 4
> +.TP 4
> +.BI \-\-leak-report= path
> +
> +Write a detailed report of all memory allocated via talloc to
> +.I path
> +.RE

Do we really need a command line option for this?  Why isn't the env var
sufficient?  This just seems to me like it clutters the interface, for
an option that is purely for debugging and should rarely if ever be used
by most users.

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 

Re: [PATCH 5/7] CLI: add --leak-report top level option

[Jameson Graef Rollins]

On Sat, Jan 19 2013, da...@tethera.net wrote:
 This roughly mimics the samba4 argument. The presence of the command
 line argument overrides any value of NOTMUCH_TALLOC_REPORT in the
 environment.
 ---
  man/man1/notmuch.1 |8 
  notmuch.c  |   18 +++---
  2 files changed, 15 insertions(+), 11 deletions(-)

 diff --git a/man/man1/notmuch.1 b/man/man1/notmuch.1
 index 6bf9b2e..5c58c41 100644
 --- a/man/man1/notmuch.1
 +++ b/man/man1/notmuch.1
 @@ -70,6 +70,14 @@ Print a synopsis of available commands and exit.
  Print the installed version of notmuch, and exit.
  .RE
  
 +.RS 4
 +.TP 4
 +.BI \-\-leak-report= path
 +
 +Write a detailed report of all memory allocated via talloc to
 +.I path
 +.RE

Do we really need a command line option for this?  Why isn't the env var
sufficient?  This just seems to me like it clutters the interface, for
an option that is purely for debugging and should rarely if ever be used
by most users.

jamie.


pgpZZK2xTThm3.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 5/7] CLI: add --leak-report top level option

[Tomi Ollila]

On Sun, Jan 20 2013, Jameson Graef Rollins jroll...@finestructure.net wrote:

 On Sat, Jan 19 2013, da...@tethera.net wrote:
 This roughly mimics the samba4 argument. The presence of the command
 line argument overrides any value of NOTMUCH_TALLOC_REPORT in the
 environment.
 ---
  man/man1/notmuch.1 |8 
  notmuch.c  |   18 +++---
  2 files changed, 15 insertions(+), 11 deletions(-)

 diff --git a/man/man1/notmuch.1 b/man/man1/notmuch.1
 index 6bf9b2e..5c58c41 100644
 --- a/man/man1/notmuch.1
 +++ b/man/man1/notmuch.1
 @@ -70,6 +70,14 @@ Print a synopsis of available commands and exit.
  Print the installed version of notmuch, and exit.
  .RE
  
 +.RS 4
 +.TP 4
 +.BI \-\-leak-report= path
 +
 +Write a detailed report of all memory allocated via talloc to
 +.I path
 +.RE

 Do we really need a command line option for this?  Why isn't the env var
 sufficient?  This just seems to me like it clutters the interface, for
 an option that is purely for debugging and should rarely if ever be used
 by most users.

Jameson does have a point. Now that we already have that environment
variable and it can be used in shipped notmuch 0.15 it is perhaps
simplest just to stick with that.

My thoughts after brief first visit to the patche series has so far
being either make the command line usage 1:1 compatible with samba
or use option like --leak-report-output=..., --leak-report-file=... or
--leak-report-to=... (and attempt to deprecate the env var...)

That said, I withdraw my previous suggestion of the command line option...

The other changes in this patch series looks initially good -- and changes
that drop deprecated features should be get in as early after last release
as possible.

 jamie.


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

[PATCH 5/7] CLI: add --leak-report top level option

[da...@tethera.net]

From: David Bremner 

This roughly mimics the samba4 argument. The presence of the command
line argument overrides any value of NOTMUCH_TALLOC_REPORT in the
environment.
---
 man/man1/notmuch.1 |8 
 notmuch.c  |   18 +++---
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/man/man1/notmuch.1 b/man/man1/notmuch.1
index 6bf9b2e..5c58c41 100644
--- a/man/man1/notmuch.1
+++ b/man/man1/notmuch.1
@@ -70,6 +70,14 @@ Print a synopsis of available commands and exit.
 Print the installed version of notmuch, and exit.
 .RE

+.RS 4
+.TP 4
+.BI \-\-leak-report= path
+
+Write a detailed report of all memory allocated via talloc to
+.I path
+.RE
+
 .SH COMMANDS


diff --git a/notmuch.c b/notmuch.c
index a674481..f8d4b35 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -250,11 +250,13 @@ main (int argc, char *argv[])
 command_t *command;
 unsigned int i;
 notmuch_bool_t print_help=FALSE, print_version=FALSE;
+const char* leak_report=NULL;
 int opt_index;

 notmuch_opt_desc_t options[] = {
{ NOTMUCH_OPT_BOOLEAN, _help, "help", 'h', 0 },
{ NOTMUCH_OPT_BOOLEAN, _version, "version", 'v', 0 },
+   { NOTMUCH_OPT_STRING, _report, "leak-report", 'l', 0 },
{ 0, 0, 0, 0, 0 }
 };

@@ -290,21 +292,15 @@ main (int argc, char *argv[])

if (strcmp (argv[opt_index], command->name) == 0) {
int ret;
-   char *talloc_report;

ret = (command->function)(local, argc - opt_index, argv + 
opt_index);

-   /* in the future support for this environment variable may
-* be supplemented or replaced by command line arguments
-* --leak-report and/or --leak-report-full */
-
-   talloc_report = getenv ("NOTMUCH_TALLOC_REPORT");
-
-   /* this relies on the previous call to
-* talloc_enable_null_tracking */
+   if (leak_report == NULL) {
+   leak_report = getenv ("NOTMUCH_TALLOC_REPORT");
+   }

-   if (talloc_report && strcmp (talloc_report, "") != 0) {
-   FILE *report = fopen (talloc_report, "w");
+   if (leak_report && (strcmp (leak_report, "") != 0)) {
+   FILE *report = fopen (leak_report, "w");
talloc_report_full (NULL, report);
}

-- 
1.7.10.4

[PATCH 5/7] CLI: add --leak-report top level option

[david]

From: David Bremner brem...@debian.org

This roughly mimics the samba4 argument. The presence of the command
line argument overrides any value of NOTMUCH_TALLOC_REPORT in the
environment.
---
 man/man1/notmuch.1 |8 
 notmuch.c  |   18 +++---
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/man/man1/notmuch.1 b/man/man1/notmuch.1
index 6bf9b2e..5c58c41 100644
--- a/man/man1/notmuch.1
+++ b/man/man1/notmuch.1
@@ -70,6 +70,14 @@ Print a synopsis of available commands and exit.
 Print the installed version of notmuch, and exit.
 .RE
 
+.RS 4
+.TP 4
+.BI \-\-leak-report= path
+
+Write a detailed report of all memory allocated via talloc to
+.I path
+.RE
+
 .SH COMMANDS
 
 
diff --git a/notmuch.c b/notmuch.c
index a674481..f8d4b35 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -250,11 +250,13 @@ main (int argc, char *argv[])
 command_t *command;
 unsigned int i;
 notmuch_bool_t print_help=FALSE, print_version=FALSE;
+const char* leak_report=NULL;
 int opt_index;
 
 notmuch_opt_desc_t options[] = {
{ NOTMUCH_OPT_BOOLEAN, print_help, help, 'h', 0 },
{ NOTMUCH_OPT_BOOLEAN, print_version, version, 'v', 0 },
+   { NOTMUCH_OPT_STRING, leak_report, leak-report, 'l', 0 },
{ 0, 0, 0, 0, 0 }
 };
 
@@ -290,21 +292,15 @@ main (int argc, char *argv[])
 
if (strcmp (argv[opt_index], command-name) == 0) {
int ret;
-   char *talloc_report;
 
ret = (command-function)(local, argc - opt_index, argv + 
opt_index);
 
-   /* in the future support for this environment variable may
-* be supplemented or replaced by command line arguments
-* --leak-report and/or --leak-report-full */
-
-   talloc_report = getenv (NOTMUCH_TALLOC_REPORT);
-
-   /* this relies on the previous call to
-* talloc_enable_null_tracking */
+   if (leak_report == NULL) {
+   leak_report = getenv (NOTMUCH_TALLOC_REPORT);
+   }
 
-   if (talloc_report  strcmp (talloc_report, ) != 0) {
-   FILE *report = fopen (talloc_report, w);
+   if (leak_report  (strcmp (leak_report, ) != 0)) {
+   FILE *report = fopen (leak_report, w);
talloc_report_full (NULL, report);
}
 
-- 
1.7.10.4

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