[PATCH 6/6] Avoid potentially dereferencing a NULL pointer

2012-09-24 Thread Justus Winter 
GMIME_IS_MULTIPART and GMIME_IS_MESSAGE both handle NULL pointers
gracefully, but the G_OBJECT_TYPE used in the error handling block
dereferences it without checking it first.

Fix this by checking whether parent->part is valid.

Found using the clang static analyzer.

Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
---
 mime-node.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mime-node.c b/mime-node.c
index 97e8b48..839737a 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -291,7 +291,7 @@ mime_node_child (mime_node_t *parent, int child)
 GMimeObject *sub;
 mime_node_t *node;

-if (!parent || child < 0 || child >= parent->nchildren)
+if (!parent || !parent->part || child < 0 || child >= parent->nchildren)
return NULL;

 if (GMIME_IS_MULTIPART (parent->part)) {
-- 
1.7.10.4

[PATCH 4/6] Fix the COERCE_STATUS macro

2012-09-24 Thread Justus Winter 
Fix the COERCE_STATUS macro to handle _internal_error being declared
as void function.

Note that the function _internal_error does not return. Evaluating to
NOTMUCH_STATUS_SUCCESS is done purely to appease the compiler.

Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
---
 lib/notmuch-private.h |   10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index bfb4111..7a409f5 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -136,13 +136,17 @@ typedef enum _notmuch_private_status {
  * to or greater than NOTMUCH_STATUS_LAST_STATUS. (The idea here is
  * that the caller has previously handled any expected
  * notmuch_private_status_t values.)
+ *
+ * Note that the function _internal_error does not return. Evaluating
+ * to NOTMUCH_STATUS_SUCCESS is done purely to appease the compiler.
  */
 #define COERCE_STATUS(private_status, format, ...) \
 ((private_status >= (notmuch_private_status_t) NOTMUCH_STATUS_LAST_STATUS)\
  ? \
- (notmuch_status_t) _internal_error (format " (%s).\n",\
- ##__VA_ARGS__,
\
- __location__) \
+ _internal_error (format " (%s).\n",   \
+  ##__VA_ARGS__,   \
+  __location__),   \
+ (notmuch_status_t) NOTMUCH_PRIVATE_STATUS_SUCCESS \
  : \
  (notmuch_status_t) private_status)

-- 
1.7.10.4

[PATCH 2/6] Provide a NORETURN_ATTRIBUTE macro similar to PRINTF_ATTRIBUTE

2012-09-24 Thread Justus Winter 
This attribute is understood by gcc since version 2.5. clang provides
support for testing for function attributes using __has_attribute. For
other compilers this macro evaluates to the empty string.

Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
---
 compat/function-attributes.h |   16 
 1 file changed, 16 insertions(+)

diff --git a/compat/function-attributes.h b/compat/function-attributes.h
index 18f9090..8450a17 100644
--- a/compat/function-attributes.h
+++ b/compat/function-attributes.h
@@ -28,4 +28,20 @@
 #define __has_attribute(x) 0
 #endif

+/* Provide a NORETURN_ATTRIBUTE macro similar to PRINTF_ATTRIBUTE from
+ * talloc.
+ *
+ * This attribute is understood by gcc since version 2.5. clang
+ * provides support for testing for function attributes.
+ */
+#ifndef NORETURN_ATTRIBUTE
+#if (__GNUC__ >= 3 ||  \
+ (__GNUC__ == 2 && __GNUC_MINOR__ >= 5) || \
+ __has_attribute (noreturn))
+#define NORETURN_ATTRIBUTE __attribute__ ((noreturn))
+#else
+#define NORETURN_ATTRIBUTE
+#endif
+#endif
+
 #endif
-- 
1.7.10.4

[PATCH 1/6] Provide a __has_attribute compatibility macro

2012-09-24 Thread Justus Winter 
__has_attribute is defined by clang and tests whether a given function
attribute is supported by clang.

Add a compatibility macro for other compilers.

Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
---
 compat/function-attributes.h |   31 +++
 1 file changed, 31 insertions(+)
 create mode 100644 compat/function-attributes.h

diff --git a/compat/function-attributes.h b/compat/function-attributes.h
new file mode 100644
index 000..18f9090
--- /dev/null
+++ b/compat/function-attributes.h
@@ -0,0 +1,31 @@
+/* function-attributes.h - Provides compiler abstractions for
+ * function attributes
+ *
+ * Copyright (c) 2012 Justus Winter <4winter at informatik.uni-hamburg.de>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ */
+
+#ifndef FUNCTION_ATTRIBUTES_H
+#define FUNCTION_ATTRIBUTES_H
+
+/* clang provides this macro to test for support for function
+ * attributes. If it isn't defined, this provides a compatibility
+ * macro for other compilers.
+ */
+#ifndef __has_attribute
+#define __has_attribute(x) 0
+#endif
+
+#endif
-- 
1.7.10.4

RFC: Annotate internal_error with the attribute noreturn 2nd patchset

2012-09-24 Thread Justus Winter 
Quoting David Bremner (2012-09-24 12:44:04)
> Justus Winter <4winter at informatik.uni-hamburg.de> writes:
> 
> > We need to discuss where those kind of macro definitions abstracting
> > away compiler differences should go. None of the files in util
> > includes notmuch-private.h, so I was unsure whether it's okay to put
> > them there.
> 
> How about a new header file in util/
> 
> function-attributes.h or something like that?
> 
> The util stuff is "lower level" than lib/ so it makes sense to me to put
> common stuff there.

Maybe we should add this file to compat?

Justus

[PATCH] Run `notmuch-show-hook' after setting `header-line-format'

2012-09-24 Thread Tomi Ollila 
On Mon, Sep 17 2012, Damien Cassou  wrote:

> This patch makes it possible for notmuch-show hooks to change the
> header line.
>
> Signed-off-by: Damien Cassou 
> ---

LGTM.

Tomi


>  emacs/notmuch-show.el |6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index ce5ea6f..16008b4 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1070,10 +1070,10 @@ function is used."
>
>(jit-lock-register #'notmuch-show-buttonise-links)
>
> -  (run-hooks 'notmuch-show-hook))
> +  ;; Set the header line to the subject of the first message.
> +  (setq header-line-format (notmuch-show-strip-re 
> (notmuch-show-get-subject)))
>
> -;; Set the header line to the subject of the first message.
> -(setq header-line-format (notmuch-show-strip-re 
> (notmuch-show-get-subject)
> +  (run-hooks 'notmuch-show-hook
>
>  (defun notmuch-show-capture-state ()
>"Capture the state of the current buffer.
> --
> 1.7.9.5
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 4/6] Fix the COERCE_STATUS macro

2012-09-24 Thread Austin Clements 
Quoth Justus Winter on Sep 24 at  5:21 pm:
> Fix the COERCE_STATUS macro to handle _internal_error being declared
> as void function.
> 
> Note that the function _internal_error does not return. Evaluating to
> NOTMUCH_STATUS_SUCCESS is done purely to appease the compiler.
> 
> Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
> ---
>  lib/notmuch-private.h |   10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
> index bfb4111..7a409f5 100644
> --- a/lib/notmuch-private.h
> +++ b/lib/notmuch-private.h
> @@ -136,13 +136,17 @@ typedef enum _notmuch_private_status {
>   * to or greater than NOTMUCH_STATUS_LAST_STATUS. (The idea here is
>   * that the caller has previously handled any expected
>   * notmuch_private_status_t values.)
> + *
> + * Note that the function _internal_error does not return. Evaluating
> + * to NOTMUCH_STATUS_SUCCESS is done purely to appease the compiler.
>   */
>  #define COERCE_STATUS(private_status, format, ...)   \
>  ((private_status >= (notmuch_private_status_t) 
> NOTMUCH_STATUS_LAST_STATUS)\
>   ?   
> \
> - (notmuch_status_t) _internal_error (format " (%s).\n",  \
> - ##__VA_ARGS__,  
> \
> - __location__)   
> \
> + _internal_error (format " (%s).\n", \
> +  ##__VA_ARGS__, \
> +  __location__), \
> + (notmuch_status_t) NOTMUCH_PRIVATE_STATUS_SUCCESS   
> \

Just a nit: why not simply NOTMUCH_STATUS_SUCCESS?

Otherwise, this series LGTM.  No need to roll another version just for
this comment.

>   :   
> \
>   (notmuch_status_t) private_status)
>

[PATCH 4/5] Annotate internal_error with the attribute noreturn

2012-09-24 Thread Justus Winter 
Annotating functions that do not return with the noreturn attribute
(which is understood by both gcc and clang) prevents static analyzers
from generating false positives (internal_error is used to terminate
the process and is used extensively in error handling code paths).

Remove the return statement that was placed there to appease the
compiler. Functions annotated with noreturn are not supposed to return
any values.

Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
---
 util/error_util.c |4 +---
 util/error_util.h |4 ++--
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/util/error_util.c b/util/error_util.c
index 630d228..d6e60fc 100644
--- a/util/error_util.c
+++ b/util/error_util.c
@@ -24,7 +24,7 @@

 #include "error_util.h"

-int
+void
 _internal_error (const char *format, ...)
 {
 va_list va_args;
@@ -35,7 +35,5 @@ _internal_error (const char *format, ...)
 vfprintf (stderr, format, va_args);

 exit (1);
-
-return 1;
 }

diff --git a/util/error_util.h b/util/error_util.h
index 27e119f..d4d4584 100644
--- a/util/error_util.h
+++ b/util/error_util.h
@@ -53,8 +53,8 @@
  *
  * Note that PRINTF_ATTRIBUTE comes from talloc.h
  */
-int
-_internal_error (const char *format, ...) PRINTF_ATTRIBUTE (1, 2);
+void
+_internal_error (const char *format, ...) PRINTF_ATTRIBUTE (1, 2) 
NORETURN_ATTRIBUTE;

 /* There's no point in continuing when we've detected that we've done
  * something wrong internally (as opposed to the user passing in a
-- 
1.7.10.4

[PATCH 3/5] Fix the COERCE_STATUS macro

2012-09-24 Thread Justus Winter 
Fix the COERCE_STATUS macro to handle _internal_error being declared
as void function.

Note that the function _internal_error does not return. Evaluating to
NOTMUCH_STATUS_SUCCESS is done purely to appease the compiler.

Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
---
 lib/notmuch-private.h |   10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index bfb4111..7a409f5 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -136,13 +136,17 @@ typedef enum _notmuch_private_status {
  * to or greater than NOTMUCH_STATUS_LAST_STATUS. (The idea here is
  * that the caller has previously handled any expected
  * notmuch_private_status_t values.)
+ *
+ * Note that the function _internal_error does not return. Evaluating
+ * to NOTMUCH_STATUS_SUCCESS is done purely to appease the compiler.
  */
 #define COERCE_STATUS(private_status, format, ...) \
 ((private_status >= (notmuch_private_status_t) NOTMUCH_STATUS_LAST_STATUS)\
  ? \
- (notmuch_status_t) _internal_error (format " (%s).\n",\
- ##__VA_ARGS__,
\
- __location__) \
+ _internal_error (format " (%s).\n",   \
+  ##__VA_ARGS__,   \
+  __location__),   \
+ (notmuch_status_t) NOTMUCH_PRIVATE_STATUS_SUCCESS \
  : \
  (notmuch_status_t) private_status)

-- 
1.7.10.4

[PATCH 2/5] RFC: Provide a NORETURN_ATTRIBUTE macro similar to PRINTF_ATTRIBUTE

2012-09-24 Thread Justus Winter 
This attribute is understood by gcc since version 2.5. clang provides
support for testing for function attributes using __has_attribute. For
other compilers this macro evaluates to the empty string.

Note: This is work in progress, please don't merge this patch. The
question that needs to be discussed is where this kind of macro should
be defined.

Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
---
 util/error_util.h |   16 
 1 file changed, 16 insertions(+)

diff --git a/util/error_util.h b/util/error_util.h
index 1b11047..27e119f 100644
--- a/util/error_util.h
+++ b/util/error_util.h
@@ -31,6 +31,22 @@
 #define __has_attribute(x) 0
 #endif

+/* Provide a NORETURN_ATTRIBUTE macro similar to PRINTF_ATTRIBUTE from
+ * talloc.
+ *
+ * This attribute is understood by gcc since version 2.5. clang
+ * provides support for testing for function attributes.
+ */
+#ifndef NORETURN_ATTRIBUTE
+#if (__GNUC__ >= 3 ||  \
+ (__GNUC__ == 2 && __GNUC_MINOR__ >= 5) || \
+ __has_attribute (noreturn))
+#define NORETURN_ATTRIBUTE __attribute__ ((noreturn))
+#else
+#define NORETURN_ATTRIBUTE
+#endif
+#endif
+
 /* There's no point in continuing when we've detected that we've done
  * something wrong internally (as opposed to the user passing in a
  * bogus value).
-- 
1.7.10.4

[PATCH 1/5] RFC: Provide a __has_attribute compatibility macro

2012-09-24 Thread Justus Winter 
__has_attribute is defined by clang and tests whether a given function
attribute is supported by clang.

Add a compatibility macro for other compilers.

Note: This is work in progress, please don't merge this patch. The
question that needs to be discussed is where this kind of macro should
be defined.

Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
---
 util/error_util.h |8 
 1 file changed, 8 insertions(+)

diff --git a/util/error_util.h b/util/error_util.h
index bb15822..1b11047 100644
--- a/util/error_util.h
+++ b/util/error_util.h
@@ -23,6 +23,14 @@

 #include 

+/* clang provides this macro to test for support for function
+ * attributes. If it isn't defined, this provides a compatibility
+ * macro for other compilers.
+ */
+#ifndef __has_attribute
+#define __has_attribute(x) 0
+#endif
+
 /* There's no point in continuing when we've detected that we've done
  * something wrong internally (as opposed to the user passing in a
  * bogus value).
-- 
1.7.10.4

RFC: Annotate internal_error with the attribute noreturn 2nd patchset

2012-09-24 Thread Justus Winter 
Thanks for your feedback. I've updated the patch series accordingly.

We need to discuss where those kind of macro definitions abstracting
away compiler differences should go. None of the files in util
includes notmuch-private.h, so I was unsure whether it's okay to put
them there.

Justus

[PATCH 0/5] Notmuch Pick (WIP or contrib)

2012-09-24 Thread David Bremner 
Mark Walters  writes:

>
> The code is not of the same standard as mainline code: in particular a
> lot of the code is written by me and is working but unidiomatic
> lisp. It has also not had widespread testing so I would expect it to
> have several bugs.

I have gotten the following traceback when running in emacs 24.2.1 if
the pick buffer is the sole window.  It might be related somehow to
running emacsclient.

Debugger entered--Lisp error: (error "Attempt to delete minibuffer or sole 
ordinary window")
  signal(error ("Attempt to delete minibuffer or sole ordinary window"))
  error("Attempt to delete minibuffer or sole ordinary window")
  byte-code(" ")
  delete-window(#>)
  notmuch-pick-close-message-window()
  notmuch-pick-quit()
  call-interactively(notmuch-pick-quit nil nil)

RFC: Annotate internal_error with the attribute noreturn 2nd patchset

2012-09-24 Thread David Bremner 
Justus Winter <4winter at informatik.uni-hamburg.de> writes:

>> The util stuff is "lower level" than lib/ so it makes sense to me to put
>> common stuff there.
>
> Maybe we should add this file to compat?

Sure, given appropriate re-wording of compat/README

d

nbook: a notmuch based address book written in python

2012-09-24 Thread Suvayu Ali 
Hi,

(I'm not subscribed to the list, so please cc: me in your replies)

I am a new notmuch user.  I use it with mutt.  I wanted a Gmail like
address book, so I used the python bindings for notmuch to write a small
python program.  This is supposed to behave like abook.  Quality
standards permitting, I would like this to be included in the contrib
directory with notmuch.  I have tested it a little, but there could be
bugs lurking somewhere.  I have created a github repository[1], which
may be treated as upstream for this program.

This is my first serious python program, and the first time I'm
contributing something significant to an open source project.  So please
let me know if I have missed something.  Looking forward to your
feedback.

Cheers,

Footnotes:

[1] 

-- 
Suvayu

Open source is the future. It sets us free.

RFC: Annotate internal_error with the attribute noreturn 2nd patchset

2012-09-24 Thread Justus Winter 
Thanks for your feedback. I've updated the patch series accordingly.

We need to discuss where those kind of macro definitions abstracting
away compiler differences should go. None of the files in util
includes notmuch-private.h, so I was unsure whether it's okay to put
them there.

Justus

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

[PATCH 4/5] Annotate internal_error with the attribute noreturn

2012-09-24 Thread Justus Winter 
Annotating functions that do not return with the noreturn attribute
(which is understood by both gcc and clang) prevents static analyzers
from generating false positives (internal_error is used to terminate
the process and is used extensively in error handling code paths).

Remove the return statement that was placed there to appease the
compiler. Functions annotated with noreturn are not supposed to return
any values.

Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
---
 util/error_util.c |4 +---
 util/error_util.h |4 ++--
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/util/error_util.c b/util/error_util.c
index 630d228..d6e60fc 100644
--- a/util/error_util.c
+++ b/util/error_util.c
@@ -24,7 +24,7 @@
 
 #include error_util.h
 
-int
+void
 _internal_error (const char *format, ...)
 {
 va_list va_args;
@@ -35,7 +35,5 @@ _internal_error (const char *format, ...)
 vfprintf (stderr, format, va_args);
 
 exit (1);
-
-return 1;
 }
 
diff --git a/util/error_util.h b/util/error_util.h
index 27e119f..d4d4584 100644
--- a/util/error_util.h
+++ b/util/error_util.h
@@ -53,8 +53,8 @@
  *
  * Note that PRINTF_ATTRIBUTE comes from talloc.h
  */
-int
-_internal_error (const char *format, ...) PRINTF_ATTRIBUTE (1, 2);
+void
+_internal_error (const char *format, ...) PRINTF_ATTRIBUTE (1, 2) 
NORETURN_ATTRIBUTE;
 
 /* There's no point in continuing when we've detected that we've done
  * something wrong internally (as opposed to the user passing in a
-- 
1.7.10.4

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

[PATCH 1/5] RFC: Provide a __has_attribute compatibility macro

2012-09-24 Thread Justus Winter 
__has_attribute is defined by clang and tests whether a given function
attribute is supported by clang.

Add a compatibility macro for other compilers.

Note: This is work in progress, please don't merge this patch. The
question that needs to be discussed is where this kind of macro should
be defined.

Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
---
 util/error_util.h |8 
 1 file changed, 8 insertions(+)

diff --git a/util/error_util.h b/util/error_util.h
index bb15822..1b11047 100644
--- a/util/error_util.h
+++ b/util/error_util.h
@@ -23,6 +23,14 @@
 
 #include talloc.h
 
+/* clang provides this macro to test for support for function
+ * attributes. If it isn't defined, this provides a compatibility
+ * macro for other compilers.
+ */
+#ifndef __has_attribute
+#define __has_attribute(x) 0
+#endif
+
 /* There's no point in continuing when we've detected that we've done
  * something wrong internally (as opposed to the user passing in a
  * bogus value).
-- 
1.7.10.4

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

[PATCH 5/5] Avoid potentially dereferencing a NULL pointer

2012-09-24 Thread Justus Winter 
GMIME_IS_MULTIPART and GMIME_IS_MESSAGE both handle NULL pointers
gracefully, but the G_OBJECT_TYPE used in the error handling block
dereferences it without checking it first.

Fix this by checking whether parent-part is valid.

Found using the clang static analyzer.

Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
---
 mime-node.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mime-node.c b/mime-node.c
index 97e8b48..839737a 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -291,7 +291,7 @@ mime_node_child (mime_node_t *parent, int child)
 GMimeObject *sub;
 mime_node_t *node;
 
-if (!parent || child  0 || child = parent-nchildren)
+if (!parent || !parent-part || child  0 || child = parent-nchildren)
return NULL;
 
 if (GMIME_IS_MULTIPART (parent-part)) {
-- 
1.7.10.4

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

[PATCH 3/5] Fix the COERCE_STATUS macro

2012-09-24 Thread Justus Winter 
Fix the COERCE_STATUS macro to handle _internal_error being declared
as void function.

Note that the function _internal_error does not return. Evaluating to
NOTMUCH_STATUS_SUCCESS is done purely to appease the compiler.

Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
---
 lib/notmuch-private.h |   10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index bfb4111..7a409f5 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -136,13 +136,17 @@ typedef enum _notmuch_private_status {
  * to or greater than NOTMUCH_STATUS_LAST_STATUS. (The idea here is
  * that the caller has previously handled any expected
  * notmuch_private_status_t values.)
+ *
+ * Note that the function _internal_error does not return. Evaluating
+ * to NOTMUCH_STATUS_SUCCESS is done purely to appease the compiler.
  */
 #define COERCE_STATUS(private_status, format, ...) \
 ((private_status = (notmuch_private_status_t) NOTMUCH_STATUS_LAST_STATUS)\
  ? \
- (notmuch_status_t) _internal_error (format  (%s).\n,\
- ##__VA_ARGS__,
\
- __location__) \
+ _internal_error (format  (%s).\n,   \
+  ##__VA_ARGS__,   \
+  __location__),   \
+ (notmuch_status_t) NOTMUCH_PRIVATE_STATUS_SUCCESS \
  : \
  (notmuch_status_t) private_status)
 
-- 
1.7.10.4

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

[PATCH 2/5] RFC: Provide a NORETURN_ATTRIBUTE macro similar to PRINTF_ATTRIBUTE

2012-09-24 Thread Justus Winter 
This attribute is understood by gcc since version 2.5. clang provides
support for testing for function attributes using __has_attribute. For
other compilers this macro evaluates to the empty string.

Note: This is work in progress, please don't merge this patch. The
question that needs to be discussed is where this kind of macro should
be defined.

Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
---
 util/error_util.h |   16 
 1 file changed, 16 insertions(+)

diff --git a/util/error_util.h b/util/error_util.h
index 1b11047..27e119f 100644
--- a/util/error_util.h
+++ b/util/error_util.h
@@ -31,6 +31,22 @@
 #define __has_attribute(x) 0
 #endif
 
+/* Provide a NORETURN_ATTRIBUTE macro similar to PRINTF_ATTRIBUTE from
+ * talloc.
+ *
+ * This attribute is understood by gcc since version 2.5. clang
+ * provides support for testing for function attributes.
+ */
+#ifndef NORETURN_ATTRIBUTE
+#if (__GNUC__ = 3 ||  \
+ (__GNUC__ == 2  __GNUC_MINOR__ = 5) || \
+ __has_attribute (noreturn))
+#define NORETURN_ATTRIBUTE __attribute__ ((noreturn))
+#else
+#define NORETURN_ATTRIBUTE
+#endif
+#endif
+
 /* There's no point in continuing when we've detected that we've done
  * something wrong internally (as opposed to the user passing in a
  * bogus value).
-- 
1.7.10.4

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

Re: Bug#688609: notmuch: Can't deal with compressed maildir files

2012-09-24 Thread David Bremner 
Joerg Jaspert jo...@debian.org writes:

 another wishlist going upstream. Would be nice if notmuch can be told
 that the mailfiles are compressed (gzip in this case), or even better -
 detect that itself. And then transparently deal with it.

Dear Joerg;

This wish is already known upstream, and there are some proposed
patches at 

http://article.gmane.org/gmane.mail.notmuch.general/11522

I'm not sure if this deals with compression or just maildir files.

I'm sure Ethan would appreciate feedback on the patches.

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

Re: RFC: Annotate internal_error with the attribute noreturn 2nd patchset

2012-09-24 Thread David Bremner 
Justus Winter 4win...@informatik.uni-hamburg.de writes:

 We need to discuss where those kind of macro definitions abstracting
 away compiler differences should go. None of the files in util
 includes notmuch-private.h, so I was unsure whether it's okay to put
 them there.

How about a new header file in util/

function-attributes.h or something like that?

The util stuff is lower level than lib/ so it makes sense to me to put
common stuff there.

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

Re: [PATCH] Run `notmuch-show-hook' after setting `header-line-format'

2012-09-24 Thread Tomi Ollila 
On Mon, Sep 17 2012, Damien Cassou damien.cas...@gmail.com wrote:

 This patch makes it possible for notmuch-show hooks to change the
 header line.

 Signed-off-by: Damien Cassou damien.cas...@gmail.com
 ---

LGTM.

Tomi


  emacs/notmuch-show.el |6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

 diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
 index ce5ea6f..16008b4 100644
 --- a/emacs/notmuch-show.el
 +++ b/emacs/notmuch-show.el
 @@ -1070,10 +1070,10 @@ function is used.

(jit-lock-register #'notmuch-show-buttonise-links)

 -  (run-hooks 'notmuch-show-hook))
 +  ;; Set the header line to the subject of the first message.
 +  (setq header-line-format (notmuch-show-strip-re 
 (notmuch-show-get-subject)))

 -;; Set the header line to the subject of the first message.
 -(setq header-line-format (notmuch-show-strip-re 
 (notmuch-show-get-subject)
 +  (run-hooks 'notmuch-show-hook

  (defun notmuch-show-capture-state ()
Capture the state of the current buffer.
 --
 1.7.9.5
 ___
 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: RFC: Annotate internal_error with the attribute noreturn 2nd patchset

2012-09-24 Thread Justus Winter 
Quoting David Bremner (2012-09-24 12:44:04)
 Justus Winter 4win...@informatik.uni-hamburg.de writes:
 
  We need to discuss where those kind of macro definitions abstracting
  away compiler differences should go. None of the files in util
  includes notmuch-private.h, so I was unsure whether it's okay to put
  them there.
 
 How about a new header file in util/
 
 function-attributes.h or something like that?
 
 The util stuff is lower level than lib/ so it makes sense to me to put
 common stuff there.

Maybe we should add this file to compat?

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

Re: [PATCH 2/2] Avoid potentially dereferencing a NULL pointer

2012-09-24 Thread Justus Winter 
Quoting Austin Clements (2012-09-22 18:19:08)
 Quoth Justus Winter on Sep 21 at  2:50 pm:
  GMIME_IS_MULTIPART and GMIME_IS_MESSAGE both handle NULL pointers
  gracefully, but the G_OBJECT_TYPE used in the error handling block
  dereferences it without checking it first.
  
  Fix this by checking whether parent-part is valid.
  
  Found using the clang static analyzer.
 
 Neat.

Yes. Besides this the code turns up no warnings (modulo one false
positive, clang doesn't understand that progress_notify is never
called if it's NULL in notmuch_database_upgrade b/c the signal handler
isn't set up then).

 Can this actually happen, though?  If so, I think this point is too
 late to be checking for a NULL part field.  It should probably be
 checked when the mime_node_t is created so that mime_node_t never has
 a NULL part field.

I'm not sure actually. Then again this patch isn't hacky at all and
being somewhat defensive isn't bad imho.

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

Re: RFC: Annotate internal_error with the attribute noreturn 2nd patchset

2012-09-24 Thread David Bremner 
Justus Winter 4win...@informatik.uni-hamburg.de writes:

 The util stuff is lower level than lib/ so it makes sense to me to put
 common stuff there.

 Maybe we should add this file to compat?

Sure, given appropriate re-wording of compat/README

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

Re: [PATCH 0/5] Notmuch Pick (WIP or contrib)

2012-09-24 Thread David Bremner 
Mark Walters markwalters1...@gmail.com writes:


 The code is not of the same standard as mainline code: in particular a
 lot of the code is written by me and is working but unidiomatic
 lisp. It has also not had widespread testing so I would expect it to
 have several bugs.

I have gotten the following traceback when running in emacs 24.2.1 if
the pick buffer is the sole window.  It might be related somehow to
running emacsclient.

Debugger entered--Lisp error: (error Attempt to delete minibuffer or sole 
ordinary window)
  signal(error (Attempt to delete minibuffer or sole ordinary window))
  error(Attempt to delete minibuffer or sole ordinary window)
  byte-code( )
  delete-window(#window 327 on *notmuch-pick-thread:000270ce*2)
  notmuch-pick-close-message-window()
  notmuch-pick-quit()
  call-interactively(notmuch-pick-quit nil nil)
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 5/6] Annotate internal_error with the attribute noreturn

2012-09-24 Thread Justus Winter 
Annotating functions that do not return with the noreturn attribute
(which is understood by both gcc and clang) prevents static analyzers
from generating false positives (internal_error is used to terminate
the process and is used extensively in error handling code paths).

Remove the return statement that was placed there to appease the
compiler. Functions annotated with noreturn are not supposed to return
any values.

Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
---
 util/error_util.c |4 +---
 util/error_util.h |6 --
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/util/error_util.c b/util/error_util.c
index 630d228..d6e60fc 100644
--- a/util/error_util.c
+++ b/util/error_util.c
@@ -24,7 +24,7 @@
 
 #include error_util.h
 
-int
+void
 _internal_error (const char *format, ...)
 {
 va_list va_args;
@@ -35,7 +35,5 @@ _internal_error (const char *format, ...)
 vfprintf (stderr, format, va_args);
 
 exit (1);
-
-return 1;
 }
 
diff --git a/util/error_util.h b/util/error_util.h
index bb15822..17c8727 100644
--- a/util/error_util.h
+++ b/util/error_util.h
@@ -23,14 +23,16 @@
 
 #include talloc.h
 
+#include function-attributes.h
+
 /* There's no point in continuing when we've detected that we've done
  * something wrong internally (as opposed to the user passing in a
  * bogus value).
  *
  * Note that PRINTF_ATTRIBUTE comes from talloc.h
  */
-int
-_internal_error (const char *format, ...) PRINTF_ATTRIBUTE (1, 2);
+void
+_internal_error (const char *format, ...) PRINTF_ATTRIBUTE (1, 2) 
NORETURN_ATTRIBUTE;
 
 /* There's no point in continuing when we've detected that we've done
  * something wrong internally (as opposed to the user passing in a
-- 
1.7.10.4

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

[PATCH 1/6] Provide a __has_attribute compatibility macro

2012-09-24 Thread Justus Winter 
__has_attribute is defined by clang and tests whether a given function
attribute is supported by clang.

Add a compatibility macro for other compilers.

Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
---
 compat/function-attributes.h |   31 +++
 1 file changed, 31 insertions(+)
 create mode 100644 compat/function-attributes.h

diff --git a/compat/function-attributes.h b/compat/function-attributes.h
new file mode 100644
index 000..18f9090
--- /dev/null
+++ b/compat/function-attributes.h
@@ -0,0 +1,31 @@
+/* function-attributes.h - Provides compiler abstractions for
+ * function attributes
+ *
+ * Copyright (c) 2012 Justus Winter 4win...@informatik.uni-hamburg.de
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ */
+
+#ifndef FUNCTION_ATTRIBUTES_H
+#define FUNCTION_ATTRIBUTES_H
+
+/* clang provides this macro to test for support for function
+ * attributes. If it isn't defined, this provides a compatibility
+ * macro for other compilers.
+ */
+#ifndef __has_attribute
+#define __has_attribute(x) 0
+#endif
+
+#endif
-- 
1.7.10.4

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

[PATCH 3/6] Extend compat/README

2012-09-24 Thread Justus Winter 
Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
---
 compat/README |7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/compat/README b/compat/README
index 38e2e14..12aacf4 100644
--- a/compat/README
+++ b/compat/README
@@ -1,6 +1,6 @@
 notmuch/compat
 
-This directory consists of two things:
+This directory consists of three things:
 
 1. Small programs used by the notmuch configure script to test for the
availability of certain system features, (library functions, etc.).
@@ -14,3 +14,8 @@ This directory consists of two things:
 
The compilation of these files is made conditional on the output of
the test programs from [1].
+
+3. Macro definitions abstracting compiler differences (e.g. function
+   attributes).
+
+   For example: function-attributes.h
-- 
1.7.10.4

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

[PATCH 4/6] Fix the COERCE_STATUS macro

2012-09-24 Thread Justus Winter 
Fix the COERCE_STATUS macro to handle _internal_error being declared
as void function.

Note that the function _internal_error does not return. Evaluating to
NOTMUCH_STATUS_SUCCESS is done purely to appease the compiler.

Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
---
 lib/notmuch-private.h |   10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index bfb4111..7a409f5 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -136,13 +136,17 @@ typedef enum _notmuch_private_status {
  * to or greater than NOTMUCH_STATUS_LAST_STATUS. (The idea here is
  * that the caller has previously handled any expected
  * notmuch_private_status_t values.)
+ *
+ * Note that the function _internal_error does not return. Evaluating
+ * to NOTMUCH_STATUS_SUCCESS is done purely to appease the compiler.
  */
 #define COERCE_STATUS(private_status, format, ...) \
 ((private_status = (notmuch_private_status_t) NOTMUCH_STATUS_LAST_STATUS)\
  ? \
- (notmuch_status_t) _internal_error (format  (%s).\n,\
- ##__VA_ARGS__,
\
- __location__) \
+ _internal_error (format  (%s).\n,   \
+  ##__VA_ARGS__,   \
+  __location__),   \
+ (notmuch_status_t) NOTMUCH_PRIVATE_STATUS_SUCCESS \
  : \
  (notmuch_status_t) private_status)
 
-- 
1.7.10.4

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

[PATCH 6/6] Avoid potentially dereferencing a NULL pointer

2012-09-24 Thread Justus Winter 
GMIME_IS_MULTIPART and GMIME_IS_MESSAGE both handle NULL pointers
gracefully, but the G_OBJECT_TYPE used in the error handling block
dereferences it without checking it first.

Fix this by checking whether parent-part is valid.

Found using the clang static analyzer.

Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
---
 mime-node.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mime-node.c b/mime-node.c
index 97e8b48..839737a 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -291,7 +291,7 @@ mime_node_child (mime_node_t *parent, int child)
 GMimeObject *sub;
 mime_node_t *node;
 
-if (!parent || child  0 || child = parent-nchildren)
+if (!parent || !parent-part || child  0 || child = parent-nchildren)
return NULL;
 
 if (GMIME_IS_MULTIPART (parent-part)) {
-- 
1.7.10.4

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

[PATCH 2/6] Provide a NORETURN_ATTRIBUTE macro similar to PRINTF_ATTRIBUTE

2012-09-24 Thread Justus Winter 
This attribute is understood by gcc since version 2.5. clang provides
support for testing for function attributes using __has_attribute. For
other compilers this macro evaluates to the empty string.

Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
---
 compat/function-attributes.h |   16 
 1 file changed, 16 insertions(+)

diff --git a/compat/function-attributes.h b/compat/function-attributes.h
index 18f9090..8450a17 100644
--- a/compat/function-attributes.h
+++ b/compat/function-attributes.h
@@ -28,4 +28,20 @@
 #define __has_attribute(x) 0
 #endif
 
+/* Provide a NORETURN_ATTRIBUTE macro similar to PRINTF_ATTRIBUTE from
+ * talloc.
+ *
+ * This attribute is understood by gcc since version 2.5. clang
+ * provides support for testing for function attributes.
+ */
+#ifndef NORETURN_ATTRIBUTE
+#if (__GNUC__ = 3 ||  \
+ (__GNUC__ == 2  __GNUC_MINOR__ = 5) || \
+ __has_attribute (noreturn))
+#define NORETURN_ATTRIBUTE __attribute__ ((noreturn))
+#else
+#define NORETURN_ATTRIBUTE
+#endif
+#endif
+
 #endif
-- 
1.7.10.4

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

Re: [PATCH 4/6] Fix the COERCE_STATUS macro

2012-09-24 Thread Austin Clements 
Quoth Justus Winter on Sep 24 at  5:21 pm:
 Fix the COERCE_STATUS macro to handle _internal_error being declared
 as void function.
 
 Note that the function _internal_error does not return. Evaluating to
 NOTMUCH_STATUS_SUCCESS is done purely to appease the compiler.
 
 Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
 ---
  lib/notmuch-private.h |   10 +++---
  1 file changed, 7 insertions(+), 3 deletions(-)
 
 diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
 index bfb4111..7a409f5 100644
 --- a/lib/notmuch-private.h
 +++ b/lib/notmuch-private.h
 @@ -136,13 +136,17 @@ typedef enum _notmuch_private_status {
   * to or greater than NOTMUCH_STATUS_LAST_STATUS. (The idea here is
   * that the caller has previously handled any expected
   * notmuch_private_status_t values.)
 + *
 + * Note that the function _internal_error does not return. Evaluating
 + * to NOTMUCH_STATUS_SUCCESS is done purely to appease the compiler.
   */
  #define COERCE_STATUS(private_status, format, ...)   \
  ((private_status = (notmuch_private_status_t) 
 NOTMUCH_STATUS_LAST_STATUS)\
   ?   
 \
 - (notmuch_status_t) _internal_error (format  (%s).\n,  \
 - ##__VA_ARGS__,  
 \
 - __location__)   
 \
 + _internal_error (format  (%s).\n, \
 +  ##__VA_ARGS__, \
 +  __location__), \
 + (notmuch_status_t) NOTMUCH_PRIVATE_STATUS_SUCCESS   
 \

Just a nit: why not simply NOTMUCH_STATUS_SUCCESS?

Otherwise, this series LGTM.  No need to roll another version just for
this comment.

   :   
 \
   (notmuch_status_t) private_status)
  
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch