Re: [notmuch] [PATCH 2/2] * avoid gcc 4.4.1 compiler warning due to ignored 'fflush' return value

Subject: Re: [notmuch] [PATCH 2/2] * avoid gcc 4.4.1 compiler warning due to ignored 'fflush' return value

Date: Tue, 01 Dec 2009 08:08:07 -0800

To: djcb@djcbsoftware.nl, notmuch@notmuchmail org

Cc:

From: Carl Worth

On Mon, 23 Nov 2009 08:21:50 +0200, Dirk-Jan C. Binnema <djcb.bulk@gmail.com> wrote:
> -#define prompt(format, ...)				\
> -    do {						\
> -	printf (format, ##__VA_ARGS__);			\
> -	fflush (stdout);				\
> -	getline (&response, &response_size, stdin);	\
> -	chomp_newline (response);			\
> +#define prompt(format, ...)					\
> +    do {							\
> +	int ignored;						\
> +	printf (format, ##__VA_ARGS__);				\
> +	fflush (stdout);					\
> +	ignored = getline (&response, &response_size, stdin);	\
> +	chomp_newline (response);				\
>      } while (0)

This patch is incorrect. Ignoring the return value of getline results in
the program invoking undefined behavior by reading uninitialized
memory. This is easily tested by, for example, typing Control-D to
provide EOF to a prompt from "notmuch setup".

How about just exiting in case of EOF as in the patch below?

I think I'll push it now.

-Carl

commit eb0cf86c7a9d5cda464d4d36a9cac66f26b5529d
Author: Carl Worth <cworth@cworth.org>
Date:   Tue Dec 1 08:06:09 2009 -0800

    notmuch setup: Exit if EOF is encountered at any prompt.
    
    If the user is explicitly providing EOF, then terminating the program
    is the most likely desired thing to do. This also avoids undefined
    behavior from continuing with an uninitialized response after ignoring
    the return value of getline().

diff --git a/notmuch-setup.c b/notmuch-setup.c
index 5ec176d..622bbaa 100644
--- a/notmuch-setup.c
+++ b/notmuch-setup.c
@@ -100,12 +100,15 @@ notmuch_setup_command (unused (void *ctx),
     unsigned int i;
     int is_new;
 
-#define prompt(format, ...)                            \
-    do {                                               \
-       printf (format, ##__VA_ARGS__);                 \
-       fflush (stdout);                                \
-       getline (&response, &response_size, stdin);     \
-       chomp_newline (response);                       \
+#define prompt(format, ...)                                    \
+    do {                                                       \
+       printf (format, ##__VA_ARGS__);                         \
+       fflush (stdout);                                        \
+       if (getline (&response, &response_size, stdin) < 0) {   \
+           printf ("Exiting.\n");                              \
+           exit (1);                                           \
+       }                                                       \
+       chomp_newline (response);                               \
     } while (0)
 
     config = notmuch_config_open (ctx, NULL, &is_new);
part-000.sig (application/pgp-signature)

Thread: