[PATCH 3/3] notmuch-restore: allocate a temporary talloc context for each line parsed.

[Jani Nikula]

On Sun, 16 Dec 2012, david at tethera.net wrote:
> From: David Bremner 
>
> This lets the high level code in notmuch restore be ignorant about
> what the lower level code is doing as far as allocating memory.
> ---
>  notmuch-restore.c |   12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/notmuch-restore.c b/notmuch-restore.c
> index 4b76d83..52e7ecb 100644
> --- a/notmuch-restore.c
> +++ b/notmuch-restore.c
> @@ -122,6 +122,7 @@ notmuch_restore_command (unused (void *ctx), int argc, 
> char *argv[])
>  char *input_file_name = NULL;
>  FILE *input = stdin;
>  char *line = NULL;
> +void *line_ctx = NULL;
>  size_t line_size;
>  ssize_t line_len;
>  
> @@ -205,10 +206,11 @@ notmuch_restore_command (unused (void *ctx), int argc, 
> char *argv[])
>  do {
>   char *query_string;

This patch only works on top of the batch tagging series, because
there's still one continue statement in the "id:" prefix check. But you
could make it work for both, *and* keep the slightly more intuitive ret
checking order if you did (yes, the slightly counter-intuitive):

if (line_ctxt != NULL)
  talloc_free (line_ctx);

right here, and...

> + line_ctx = talloc_new (ctx);
>   if (input_format == DUMP_FORMAT_SUP) {
> - ret = parse_sup_line (ctx, line, _string, tag_ops);
> + ret = parse_sup_line (line_ctx, line, _string, tag_ops);
>   } else {
> - ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS,
> + ret = parse_tag_line (line_ctx, line, TAG_FLAG_BE_GENEROUS,
> _string, tag_ops);
>  
>   if (ret == 0) {
> @@ -238,8 +240,14 @@ notmuch_restore_command (unused (void *ctx), int argc, 
> char *argv[])
>   break;
>   }
>  
> + talloc_free (line_ctx);
> + /* setting to NULL is important to avoid a double free */
> + line_ctx = NULL;

...removed the above lines here.

Otherwise I think you'll need a temporary goto until the batch tagging
series. (I'm fine with that too.)

Overall the series LGTM, and I like how this dodges the query_string
alloc/free problem altogether.

BR,
Jani.

>  }  while ((line_len = getline (, _size, input)) != -1);
>  
> +if (line_ctx != NULL)
> + talloc_free (line_ctx);
> +
>  if (input_format == DUMP_FORMAT_SUP)
>   regfree ();
>  
> -- 
> 1.7.10.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 3/3] notmuch-restore: allocate a temporary talloc context for each line parsed.

[da...@tethera.net]

From: David Bremner 

This lets the high level code in notmuch restore be ignorant about
what the lower level code is doing as far as allocating memory.
---
 notmuch-restore.c |   12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index 665373f..9ed9b51 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -125,6 +125,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char 
*argv[])
 char *input_file_name = NULL;
 FILE *input = stdin;
 char *line = NULL;
+void *line_ctx = NULL;
 size_t line_size;
 ssize_t line_len;

@@ -208,10 +209,14 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
 do {
char *query_string;

+   if (line_ctx != NULL)
+   talloc_free (line_ctx);
+
+   line_ctx = talloc_new (ctx);
if (input_format == DUMP_FORMAT_SUP) {
-   ret = parse_sup_line (ctx, line, _string, tag_ops);
+   ret = parse_sup_line (line_ctx, line, _string, tag_ops);
} else {
-   ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS,
+   ret = parse_tag_line (line_ctx, line, TAG_FLAG_BE_GENEROUS,
  _string, tag_ops);

if (ret == 0) {
@@ -244,6 +249,9 @@ notmuch_restore_command (unused (void *ctx), int argc, char 
*argv[])

 }  while ((line_len = getline (, _size, input)) != -1);

+if (line_ctx != NULL)
+   talloc_free (line_ctx);
+
 if (input_format == DUMP_FORMAT_SUP)
regfree ();

-- 
1.7.10.4

[PATCH 3/3] notmuch-restore: allocate a temporary talloc context for each line parsed.

[da...@tethera.net]

From: David Bremner 

This lets the high level code in notmuch restore be ignorant about
what the lower level code is doing as far as allocating memory.
---
 notmuch-restore.c |   12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index 4b76d83..52e7ecb 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -122,6 +122,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char 
*argv[])
 char *input_file_name = NULL;
 FILE *input = stdin;
 char *line = NULL;
+void *line_ctx = NULL;
 size_t line_size;
 ssize_t line_len;

@@ -205,10 +206,11 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
 do {
char *query_string;

+   line_ctx = talloc_new (ctx);
if (input_format == DUMP_FORMAT_SUP) {
-   ret = parse_sup_line (ctx, line, _string, tag_ops);
+   ret = parse_sup_line (line_ctx, line, _string, tag_ops);
} else {
-   ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS,
+   ret = parse_tag_line (line_ctx, line, TAG_FLAG_BE_GENEROUS,
  _string, tag_ops);

if (ret == 0) {
@@ -238,8 +240,14 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
break;
}

+   talloc_free (line_ctx);
+   /* setting to NULL is important to avoid a double free */
+   line_ctx = NULL;
 }  while ((line_len = getline (, _size, input)) != -1);

+if (line_ctx != NULL)
+   talloc_free (line_ctx);
+
 if (input_format == DUMP_FORMAT_SUP)
regfree ();

-- 
1.7.10.4

[PATCH 3/3] notmuch-restore: allocate a temporary talloc context for each line parsed.

[david]

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

This lets the high level code in notmuch restore be ignorant about
what the lower level code is doing as far as allocating memory.
---
 notmuch-restore.c |   12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index 4b76d83..52e7ecb 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -122,6 +122,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char 
*argv[])
 char *input_file_name = NULL;
 FILE *input = stdin;
 char *line = NULL;
+void *line_ctx = NULL;
 size_t line_size;
 ssize_t line_len;
 
@@ -205,10 +206,11 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
 do {
char *query_string;
 
+   line_ctx = talloc_new (ctx);
if (input_format == DUMP_FORMAT_SUP) {
-   ret = parse_sup_line (ctx, line, query_string, tag_ops);
+   ret = parse_sup_line (line_ctx, line, query_string, tag_ops);
} else {
-   ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS,
+   ret = parse_tag_line (line_ctx, line, TAG_FLAG_BE_GENEROUS,
  query_string, tag_ops);
 
if (ret == 0) {
@@ -238,8 +240,14 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
break;
}
 
+   talloc_free (line_ctx);
+   /* setting to NULL is important to avoid a double free */
+   line_ctx = NULL;
 }  while ((line_len = getline (line, line_size, input)) != -1);
 
+if (line_ctx != NULL)
+   talloc_free (line_ctx);
+
 if (input_format == DUMP_FORMAT_SUP)
regfree (regex);
 
-- 
1.7.10.4

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

Re: [PATCH 3/3] notmuch-restore: allocate a temporary talloc context for each line parsed.

[Jani Nikula]

On Sun, 16 Dec 2012, da...@tethera.net wrote:
 From: David Bremner brem...@debian.org

 This lets the high level code in notmuch restore be ignorant about
 what the lower level code is doing as far as allocating memory.
 ---
  notmuch-restore.c |   12 ++--
  1 file changed, 10 insertions(+), 2 deletions(-)

 diff --git a/notmuch-restore.c b/notmuch-restore.c
 index 4b76d83..52e7ecb 100644
 --- a/notmuch-restore.c
 +++ b/notmuch-restore.c
 @@ -122,6 +122,7 @@ notmuch_restore_command (unused (void *ctx), int argc, 
 char *argv[])
  char *input_file_name = NULL;
  FILE *input = stdin;
  char *line = NULL;
 +void *line_ctx = NULL;
  size_t line_size;
  ssize_t line_len;
  
 @@ -205,10 +206,11 @@ notmuch_restore_command (unused (void *ctx), int argc, 
 char *argv[])
  do {
   char *query_string;

This patch only works on top of the batch tagging series, because
there's still one continue statement in the id: prefix check. But you
could make it work for both, *and* keep the slightly more intuitive ret
checking order if you did (yes, the slightly counter-intuitive):

if (line_ctxt != NULL)
  talloc_free (line_ctx);

right here, and...

 + line_ctx = talloc_new (ctx);
   if (input_format == DUMP_FORMAT_SUP) {
 - ret = parse_sup_line (ctx, line, query_string, tag_ops);
 + ret = parse_sup_line (line_ctx, line, query_string, tag_ops);
   } else {
 - ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS,
 + ret = parse_tag_line (line_ctx, line, TAG_FLAG_BE_GENEROUS,
 query_string, tag_ops);
  
   if (ret == 0) {
 @@ -238,8 +240,14 @@ notmuch_restore_command (unused (void *ctx), int argc, 
 char *argv[])
   break;
   }
  
 + talloc_free (line_ctx);
 + /* setting to NULL is important to avoid a double free */
 + line_ctx = NULL;

...removed the above lines here.

Otherwise I think you'll need a temporary goto until the batch tagging
series. (I'm fine with that too.)

Overall the series LGTM, and I like how this dodges the query_string
alloc/free problem altogether.

BR,
Jani.

  }  while ((line_len = getline (line, line_size, input)) != -1);
  
 +if (line_ctx != NULL)
 + talloc_free (line_ctx);
 +
  if (input_format == DUMP_FORMAT_SUP)
   regfree (regex);
  
 -- 
 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

[PATCH 3/3] notmuch-restore: allocate a temporary talloc context for each line parsed.

[david]

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

This lets the high level code in notmuch restore be ignorant about
what the lower level code is doing as far as allocating memory.
---
 notmuch-restore.c |   12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index 665373f..9ed9b51 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -125,6 +125,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char 
*argv[])
 char *input_file_name = NULL;
 FILE *input = stdin;
 char *line = NULL;
+void *line_ctx = NULL;
 size_t line_size;
 ssize_t line_len;
 
@@ -208,10 +209,14 @@ notmuch_restore_command (unused (void *ctx), int argc, 
char *argv[])
 do {
char *query_string;
 
+   if (line_ctx != NULL)
+   talloc_free (line_ctx);
+
+   line_ctx = talloc_new (ctx);
if (input_format == DUMP_FORMAT_SUP) {
-   ret = parse_sup_line (ctx, line, query_string, tag_ops);
+   ret = parse_sup_line (line_ctx, line, query_string, tag_ops);
} else {
-   ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS,
+   ret = parse_tag_line (line_ctx, line, TAG_FLAG_BE_GENEROUS,
  query_string, tag_ops);
 
if (ret == 0) {
@@ -244,6 +249,9 @@ notmuch_restore_command (unused (void *ctx), int argc, char 
*argv[])
 
 }  while ((line_len = getline (line, line_size, input)) != -1);
 
+if (line_ctx != NULL)
+   talloc_free (line_ctx);
+
 if (input_format == DUMP_FORMAT_SUP)
regfree (regex);
 
-- 
1.7.10.4

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