[PATCH 1/4] cli: fix use of uninitialized variable in "notmuch reply"

2012-01-07 Thread Mark Walters 

Hello

I do not get the failure with just 1/4 applied but do with all 4
applied. The trivial patch below fixes it, but it might not be the best
solution.

The failure occurs because Jani's patch changes the behavior of a couple
of emacs/notmuch internal functions: the function
notmuch-search-reply-to-thread is renamed to
notmuch-search-reply-all-to-thread and a new
notmuch-search-reply-to-thread function is added (which does reply to
sender) and a similar change for notmuch-show-reply-.. . Since the
keybindings are also remapped the user will not notice any difference.

However, if the user has any key-bindings etc in their .emacs file the
behaviour could change. It might be preferable to keep the existing
functions as they are and give the new reply-to-sender functions a new
name.

Best wishes

Mark


diff --git a/test/emacs b/test/emacs
index a06c223..5047d46 100755
--- a/test/emacs
+++ b/test/emacs
@@ -258,7 +258,7 @@ test_expect_equal_file OUTPUT EXPECTED
 test_begin_subtest "Reply within emacs"
 test_emacs '(notmuch-search "subject:\"testing message sent via SMTP\"")
(notmuch-test-wait)
-   (notmuch-search-reply-to-thread)
+   (notmuch-search-reply-all-to-thread)
(test-output)'
 sed -i -e 's/^In-Reply-To: <.*>$/In-Reply-To: /' OUTPUT
 cat

[PATCH 1/4] cli: fix use of uninitialized variable in "notmuch reply"

2012-01-07 Thread Jani Nikula 
On Jan 7, 2012 5:52 AM, "David Bremner"  wrote:
>
> On Thu,  5 Jan 2012 22:25:12 +0200, Jani Nikula  wrote:
> > notmuch_show_params_t params is only initialized partially in
> > notmuch_reply_command(). The only field that is used uninitialized is
> > params.decrypt. It is usually non-zero, making "notmuch reply" on
encrypted
> > messages work by coincidence.
>
> Hi Jani;
>
> I get one test failure with this patch on current master:

Can't investigate right now, but did you try with just patch 1/4? (I really
should have separated the bug fix from the rest.)

J.

>
>  FAIL   Reply within emacs
>--- emacs.24.expected   2012-01-07 03:47:50.0 +
>+++ emacs.24.output 2012-01-07 03:47:50.0 +
>@@ -1,5 +1,5 @@
> From: Notmuch Test Suite 
>-To: user at example.com
>+To:
> Subject: Re: Testing message sent via SMTP
> In-Reply-To: 
> Fcc: /tmp/notmuch-dev-bremner/test/tmp.emacs/mail/sent
>
> d
-- next part --
An HTML attachment was scrubbed...
URL:

[PATCH 1/4] cli: fix use of uninitialized variable in "notmuch reply"

2012-01-07 Thread David Bremner 
On Sat, 7 Jan 2012 09:31:35 +0200, Jani Nikula  wrote:
> On Jan 7, 2012 5:52 AM, "David Bremner"  wrote:
> >
> > On Thu,  5 Jan 2012 22:25:12 +0200, Jani Nikula  wrote:
> > > notmuch_show_params_t params is only initialized partially in
> > > notmuch_reply_command(). The only field that is used uninitialized is
> > > params.decrypt. It is usually non-zero, making "notmuch reply" on
> encrypted
> > > messages work by coincidence.
> >
> > Hi Jani;
> >
> > I get one test failure with this patch on current master:
> 
> Can't investigate right now, but did you try with just patch 1/4? (I really
> should have separated the bug fix from the rest.)
> 
> J.
> 

Hi Jani;

I _thought_ I was applying just that one patch, but I can't duplicate
the error now, so I might have messed up using dme's new "patch
application wizard". So, nevermind, sorry for the noise.

d

Re: [PATCH 1/4] cli: fix use of uninitialized variable in notmuch reply

2012-01-07 Thread Mark Walters 

Hello

I do not get the failure with just 1/4 applied but do with all 4
applied. The trivial patch below fixes it, but it might not be the best
solution.

The failure occurs because Jani's patch changes the behavior of a couple
of emacs/notmuch internal functions: the function
notmuch-search-reply-to-thread is renamed to
notmuch-search-reply-all-to-thread and a new
notmuch-search-reply-to-thread function is added (which does reply to
sender) and a similar change for notmuch-show-reply-.. . Since the
keybindings are also remapped the user will not notice any difference.

However, if the user has any key-bindings etc in their .emacs file the
behaviour could change. It might be preferable to keep the existing
functions as they are and give the new reply-to-sender functions a new
name.

Best wishes

Mark


diff --git a/test/emacs b/test/emacs
index a06c223..5047d46 100755
--- a/test/emacs
+++ b/test/emacs
@@ -258,7 +258,7 @@ test_expect_equal_file OUTPUT EXPECTED
 test_begin_subtest Reply within emacs
 test_emacs '(notmuch-search subject:\testing message sent via SMTP\)
(notmuch-test-wait)
-   (notmuch-search-reply-to-thread)
+   (notmuch-search-reply-all-to-thread)
(test-output)'
 sed -i -e 's/^In-Reply-To: .*$/In-Reply-To: XXX/' OUTPUT
 cat EOF EXPECTED



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

Re: [PATCH 1/4] cli: fix use of uninitialized variable in notmuch reply

2012-01-07 Thread David Bremner 
On Sat, 7 Jan 2012 09:31:35 +0200, Jani Nikula j...@nikula.org wrote:
 On Jan 7, 2012 5:52 AM, David Bremner da...@tethera.net wrote:
 
  On Thu,  5 Jan 2012 22:25:12 +0200, Jani Nikula j...@nikula.org wrote:
   notmuch_show_params_t params is only initialized partially in
   notmuch_reply_command(). The only field that is used uninitialized is
   params.decrypt. It is usually non-zero, making notmuch reply on
 encrypted
   messages work by coincidence.
 
  Hi Jani;
 
  I get one test failure with this patch on current master:
 
 Can't investigate right now, but did you try with just patch 1/4? (I really
 should have separated the bug fix from the rest.)
 
 J.
 

Hi Jani;

I _thought_ I was applying just that one patch, but I can't duplicate
the error now, so I might have messed up using dme's new patch
application wizard. So, nevermind, sorry for the noise.

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

[PATCH 1/4] cli: fix use of uninitialized variable in "notmuch reply"

2012-01-06 Thread David Bremner 
On Thu,  5 Jan 2012 22:25:12 +0200, Jani Nikula  wrote:
> notmuch_show_params_t params is only initialized partially in
> notmuch_reply_command(). The only field that is used uninitialized is
> params.decrypt. It is usually non-zero, making "notmuch reply" on encrypted
> messages work by coincidence.

Hi Jani;

I get one test failure with this patch on current master:

 FAIL   Reply within emacs
--- emacs.24.expected   2012-01-07 03:47:50.0 +
+++ emacs.24.output 2012-01-07 03:47:50.0 +
@@ -1,5 +1,5 @@
 From: Notmuch Test Suite 
-To: user at example.com
+To: 
 Subject: Re: Testing message sent via SMTP
 In-Reply-To: 
 Fcc: /tmp/notmuch-dev-bremner/test/tmp.emacs/mail/sent

d

[PATCH 1/4] cli: fix use of uninitialized variable in "notmuch reply"

2012-01-06 Thread Jani Nikula 
On Thu, 05 Jan 2012 23:22:34 -0400, David Bremner  wrote:
> On Thu,  5 Jan 2012 22:25:12 +0200, Jani Nikula  wrote:
> > -notmuch_show_params_t params;
> > +notmuch_show_params_t params = { .part = -1 };
> >  
> >  reply_format_func = notmuch_reply_format_default;
> > -params.part = -1;
> 
> Do I understand correctly that this is just a style change, or do you
> rely on some c99(?) behaviour of initializing the other elements to 0?

It is not just a style change, and initializing a struct partially
initializes the rest of the elements to 0. That's not C99 specific
behaviour, though using the designated initializer to initialize .part
to -1 is.

All of these result in the same value for params:

notmuch_show_params_t params = { .part = -1 };

notmuch_show_params_t params = { 0, 0, -1 };

notmuch_show_params_t params = { 0, 0, -1, NULL, 0 };

notmuch_show_params_t params = { 0 };
params.part = -1;

IMHO the first is cleanest and unaffected by changes in
notmuch_show_params_t, though might be surprising if you're not (yet)
used to C99 designated initializers.

In any case, in the current implementation only .part and .cryptoctx are
initialized, and the rest of the fields are random. This needs to be
fixed one way or another.


BR,
Jani.

[PATCH 1/4] cli: fix use of uninitialized variable in "notmuch reply"

2012-01-06 Thread David Bremner 
On Fri, 06 Jan 2012 10:11:42 +0200, Jani Nikula  wrote:

> IMHO the first is cleanest and unaffected by changes in
> notmuch_show_params_t, though might be surprising if you're not (yet)
> used to C99 designated initializers.

Obviously not ;). It would nice to have a consensus about the use of C99
features in the notmuch code. Personally I think it's fine, but I know
we have had some back and forth about e.g. mid-block variable declarations.

> In any case, in the current implementation only .part and .cryptoctx are
> initialized, and the rest of the fields are random. This needs to be
> fixed one way or another.

Agreed.

d

Re: [PATCH 1/4] cli: fix use of uninitialized variable in notmuch reply

2012-01-06 Thread Jani Nikula 
On Thu, 05 Jan 2012 23:22:34 -0400, David Bremner da...@tethera.net wrote:
 On Thu,  5 Jan 2012 22:25:12 +0200, Jani Nikula j...@nikula.org wrote:
  -notmuch_show_params_t params;
  +notmuch_show_params_t params = { .part = -1 };
   
   reply_format_func = notmuch_reply_format_default;
  -params.part = -1;
 
 Do I understand correctly that this is just a style change, or do you
 rely on some c99(?) behaviour of initializing the other elements to 0?

It is not just a style change, and initializing a struct partially
initializes the rest of the elements to 0. That's not C99 specific
behaviour, though using the designated initializer to initialize .part
to -1 is.

All of these result in the same value for params:

notmuch_show_params_t params = { .part = -1 };

notmuch_show_params_t params = { 0, 0, -1 };

notmuch_show_params_t params = { 0, 0, -1, NULL, 0 };

notmuch_show_params_t params = { 0 };
params.part = -1;

IMHO the first is cleanest and unaffected by changes in
notmuch_show_params_t, though might be surprising if you're not (yet)
used to C99 designated initializers.

In any case, in the current implementation only .part and .cryptoctx are
initialized, and the rest of the fields are random. This needs to be
fixed one way or another.


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

Re: [PATCH 1/4] cli: fix use of uninitialized variable in notmuch reply

2012-01-06 Thread David Bremner 
On Fri, 06 Jan 2012 10:11:42 +0200, Jani Nikula j...@nikula.org wrote:

 IMHO the first is cleanest and unaffected by changes in
 notmuch_show_params_t, though might be surprising if you're not (yet)
 used to C99 designated initializers.

Obviously not ;). It would nice to have a consensus about the use of C99
features in the notmuch code. Personally I think it's fine, but I know
we have had some back and forth about e.g. mid-block variable declarations.

 In any case, in the current implementation only .part and .cryptoctx are
 initialized, and the rest of the fields are random. This needs to be
 fixed one way or another.

Agreed.

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

Re: [PATCH 1/4] cli: fix use of uninitialized variable in notmuch reply

2012-01-06 Thread David Bremner 
On Thu,  5 Jan 2012 22:25:12 +0200, Jani Nikula j...@nikula.org wrote:
 notmuch_show_params_t params is only initialized partially in
 notmuch_reply_command(). The only field that is used uninitialized is
 params.decrypt. It is usually non-zero, making notmuch reply on encrypted
 messages work by coincidence.

Hi Jani;

I get one test failure with this patch on current master:

 FAIL   Reply within emacs
--- emacs.24.expected   2012-01-07 03:47:50.0 +
+++ emacs.24.output 2012-01-07 03:47:50.0 +
@@ -1,5 +1,5 @@
 From: Notmuch Test Suite test_su...@notmuchmail.org
-To: u...@example.com
+To: 
 Subject: Re: Testing message sent via SMTP
 In-Reply-To: XXX
 Fcc: /tmp/notmuch-dev-bremner/test/tmp.emacs/mail/sent

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

Re: [PATCH 1/4] cli: fix use of uninitialized variable in notmuch reply

2012-01-06 Thread Jani Nikula 
On Jan 7, 2012 5:52 AM, David Bremner da...@tethera.net wrote:

 On Thu,  5 Jan 2012 22:25:12 +0200, Jani Nikula j...@nikula.org wrote:
  notmuch_show_params_t params is only initialized partially in
  notmuch_reply_command(). The only field that is used uninitialized is
  params.decrypt. It is usually non-zero, making notmuch reply on
encrypted
  messages work by coincidence.

 Hi Jani;

 I get one test failure with this patch on current master:

Can't investigate right now, but did you try with just patch 1/4? (I really
should have separated the bug fix from the rest.)

J.


  FAIL   Reply within emacs
--- emacs.24.expected   2012-01-07 03:47:50.0 +
+++ emacs.24.output 2012-01-07 03:47:50.0 +
@@ -1,5 +1,5 @@
 From: Notmuch Test Suite test_su...@notmuchmail.org
-To: u...@example.com
+To:
 Subject: Re: Testing message sent via SMTP
 In-Reply-To: XXX
 Fcc: /tmp/notmuch-dev-bremner/test/tmp.emacs/mail/sent

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

[PATCH 1/4] cli: fix use of uninitialized variable in "notmuch reply"

2012-01-05 Thread David Bremner 
On Thu,  5 Jan 2012 22:25:12 +0200, Jani Nikula  wrote:
> -notmuch_show_params_t params;
> +notmuch_show_params_t params = { .part = -1 };
>  
>  reply_format_func = notmuch_reply_format_default;
> -params.part = -1;

Do I understand correctly that this is just a style change, or do you
rely on some c99(?) behaviour of initializing the other elements to 0?

d

[PATCH 1/4] cli: fix use of uninitialized variable in "notmuch reply"

2012-01-05 Thread Jani Nikula 
notmuch_show_params_t params is only initialized partially in
notmuch_reply_command(). The only field that is used uninitialized is
params.decrypt. It is usually non-zero, making "notmuch reply" on encrypted
messages work by coincidence.

Initialize params properly, and set params.decrypt as needed.

Signed-off-by: Jani Nikula 
---
 notmuch-reply.c |   10 +-
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index f8d5f64..1f33a86 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -621,11 +621,9 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
 char *opt, *query_string;
 int i, ret = 0;
 int (*reply_format_func)(void *ctx, notmuch_config_t *config, 
notmuch_query_t *query, notmuch_show_params_t *params);
-notmuch_show_params_t params;
+notmuch_show_params_t params = { .part = -1 };

 reply_format_func = notmuch_reply_format_default;
-params.part = -1;
-params.cryptoctx = NULL;

 argc--; argv++; /* skip subcommand argument */

@@ -647,10 +645,12 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
} else if ((STRNCMP_LITERAL (argv[i], "--decrypt") == 0)) {
if (params.cryptoctx == NULL) {
GMimeSession* session = g_object_new(g_mime_session_get_type(), 
NULL);
-   if (NULL == (params.cryptoctx = g_mime_gpg_context_new(session, 
"gpg")))
+   if (NULL == (params.cryptoctx = g_mime_gpg_context_new(session, 
"gpg"))) {
fprintf (stderr, "Failed to construct gpg context.\n");
-   else
+   } else {
+   params.decrypt = TRUE;

g_mime_gpg_context_set_always_trust((GMimeGpgContext*)params.cryptoctx, FALSE);
+   }
g_object_unref (session);
session = NULL;
}
-- 
1.7.5.4

[PATCH 1/4] cli: fix use of uninitialized variable in notmuch reply

2012-01-05 Thread Jani Nikula 
notmuch_show_params_t params is only initialized partially in
notmuch_reply_command(). The only field that is used uninitialized is
params.decrypt. It is usually non-zero, making notmuch reply on encrypted
messages work by coincidence.

Initialize params properly, and set params.decrypt as needed.

Signed-off-by: Jani Nikula j...@nikula.org
---
 notmuch-reply.c |   10 +-
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index f8d5f64..1f33a86 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -621,11 +621,9 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
 char *opt, *query_string;
 int i, ret = 0;
 int (*reply_format_func)(void *ctx, notmuch_config_t *config, 
notmuch_query_t *query, notmuch_show_params_t *params);
-notmuch_show_params_t params;
+notmuch_show_params_t params = { .part = -1 };
 
 reply_format_func = notmuch_reply_format_default;
-params.part = -1;
-params.cryptoctx = NULL;
 
 argc--; argv++; /* skip subcommand argument */
 
@@ -647,10 +645,12 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
} else if ((STRNCMP_LITERAL (argv[i], --decrypt) == 0)) {
if (params.cryptoctx == NULL) {
GMimeSession* session = g_object_new(g_mime_session_get_type(), 
NULL);
-   if (NULL == (params.cryptoctx = g_mime_gpg_context_new(session, 
gpg)))
+   if (NULL == (params.cryptoctx = g_mime_gpg_context_new(session, 
gpg))) {
fprintf (stderr, Failed to construct gpg context.\n);
-   else
+   } else {
+   params.decrypt = TRUE;

g_mime_gpg_context_set_always_trust((GMimeGpgContext*)params.cryptoctx, FALSE);
+   }
g_object_unref (session);
session = NULL;
}
-- 
1.7.5.4

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

Re: [PATCH 1/4] cli: fix use of uninitialized variable in notmuch reply

2012-01-05 Thread David Bremner 
On Thu,  5 Jan 2012 22:25:12 +0200, Jani Nikula j...@nikula.org wrote:
 -notmuch_show_params_t params;
 +notmuch_show_params_t params = { .part = -1 };
  
  reply_format_func = notmuch_reply_format_default;
 -params.part = -1;

Do I understand correctly that this is just a style change, or do you
rely on some c99(?) behaviour of initializing the other elements to 0?

d

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