From 564787560c432d02dfc311ec3489f227d2a56abb Mon Sep 17 00:00:00 2001 From: Michael Elkins Date: Wed, 10 Apr 2013 22:38:47 +0000 Subject: [PATCH] fix various compiler warnings; most were due to unchecked return values from system calls. --- browser.c | 30 ++++++++++++++++++++++++++---- enter.c | 4 +++- imap/message.c | 6 ++++-- init.c | 2 -- mbox.c | 6 +++++- muttlib.c | 9 +++++++-- mx.c | 8 +++++++- pop.c | 21 +++++++++++++++++---- query.c | 7 ++++++- smime.c | 18 +++++++++++++++++- system.c | 3 ++- 11 files changed, 94 insertions(+), 20 deletions(-) diff --git a/browser.c b/browser.c index cda4900a..3d69b9d4 100644 --- a/browser.c +++ b/browser.c @@ -615,7 +615,11 @@ void _mutt_select_file (char *f, size_t flen, int flags, char ***files, int *num } else { - getcwd (LastDir, sizeof (LastDir)); + if (getcwd (LastDir, sizeof (LastDir)) == NULL) + { + dprint(1, (debugfile, "%s:%d getcwd() returned NULL\n", __FILE__, __LINE__)); + LastDir[0] = '\0'; + } safe_strcat (LastDir, sizeof (LastDir), "/"); safe_strncat (LastDir, sizeof (LastDir), f, i); } @@ -625,7 +629,13 @@ void _mutt_select_file (char *f, size_t flen, int flags, char ***files, int *num if (f[0] == '/') strcpy (LastDir, "/"); /* __STRCPY_CHECKED__ */ else - getcwd (LastDir, sizeof (LastDir)); + { + if (getcwd (LastDir, sizeof (LastDir)) == NULL) + { + dprint(1, (debugfile, "%s:%d getcwd() returned NULL\n", __FILE__, __LINE__)); + LastDir[0] = '\0'; + } + } } if (i <= 0 && f[0] != '/') @@ -640,7 +650,13 @@ void _mutt_select_file (char *f, size_t flen, int flags, char ***files, int *num else { if (!folder) - getcwd (LastDir, sizeof (LastDir)); + { + if (getcwd (LastDir, sizeof (LastDir)) == NULL) + { + dprint(1, (debugfile, "%s:%d getcwd() returned NULL\n", __FILE__, __LINE__)); + LastDir[0] = '\0'; + } + } else if (!LastDir[0]) strfcpy (LastDir, NONULL(Maildir), sizeof (LastDir)); @@ -659,7 +675,13 @@ void _mutt_select_file (char *f, size_t flen, int flags, char ***files, int *num while (i && LastDir[--i] == '/') LastDir[i] = '\0'; if (!LastDir[0]) - getcwd (LastDir, sizeof (LastDir)); + { + if (getcwd (LastDir, sizeof (LastDir)) == NULL) + { + dprint(1, (debugfile, "%s:%d getcwd() returned NULL\n", __FILE__, __LINE__)); + LastDir[0] = '\0'; + } + } } } diff --git a/enter.c b/enter.c index 077c1e63..71214104 100644 --- a/enter.c +++ b/enter.c @@ -28,6 +28,7 @@ #include "history.h" #include +#include /* redraw flags for mutt_enter_string() */ enum @@ -94,7 +95,8 @@ static void my_wcstombs (char *dest, size_t dlen, const wchar_t *src, size_t sle /* If this works, we can stop now */ if (dlen >= MB_LEN_MAX) { - wcrtomb (dest, 0, &st); + if (wcrtomb (dest, 0, &st) == (size_t) -1) + dprint(1, (debugfile, "%s:%d wcrtomb() returned -1, errno=%d\n", __FILE__, __LINE__, errno)); return; } diff --git a/imap/message.c b/imap/message.c index 742ac8cf..0b81a706 100644 --- a/imap/message.c +++ b/imap/message.c @@ -561,11 +561,13 @@ int imap_fetch_message (MESSAGE *msg, CONTEXT *ctx, int msgno) } h->lines = 0; - fgets (buf, sizeof (buf), msg->fp); + if (fgets (buf, sizeof (buf), msg->fp) == NULL) + ; /* EOF checked below */ while (!feof (msg->fp)) { h->lines++; - fgets (buf, sizeof (buf), msg->fp); + if (fgets (buf, sizeof (buf), msg->fp) == NULL) + ; /* EOF checked in while loop condition */ } h->content->length = ftell (msg->fp) - h->content->offset; diff --git a/init.c b/init.c index a3ec2325..0cc362b7 100644 --- a/init.c +++ b/init.c @@ -2822,7 +2822,6 @@ int mutt_getvaluebyname (const char *name, const struct mapping_t *map) #ifdef DEBUG static void start_debug (void) { - time_t t; int i; char buf[_POSIX_PATH_MAX]; char buf2[_POSIX_PATH_MAX]; @@ -2836,7 +2835,6 @@ static void start_debug (void) } if ((debugfile = safe_fopen(buf, "w")) != NULL) { - t = time (0); setbuf (debugfile, NULL); /* don't buffer the debugging output! */ dprint(1,(debugfile,"Mutt/%s (%s) debugging at level %d\n", MUTT_VERSION, ReleaseDate, debuglevel)); diff --git a/mbox.c b/mbox.c index 2af4dd22..e69870d4 100644 --- a/mbox.c +++ b/mbox.c @@ -968,7 +968,11 @@ int mbox_sync_mailbox (CONTEXT *ctx, int *index_hint) if (i == 0) { ctx->size = ftello (ctx->fp); /* update the size of the mailbox */ - ftruncate (fileno (ctx->fp), ctx->size); + if (ftruncate (fileno (ctx->fp), ctx->size) == -1) + { + dprint(1, (debugfile, "%s:%d ftrunctate() returned -1, errno=%d\n", __FILE__, __LINE__, errno)); + i = -1; + } } } diff --git a/muttlib.c b/muttlib.c index 7e93a3a2..62fedd84 100644 --- a/muttlib.c +++ b/muttlib.c @@ -67,6 +67,7 @@ void mutt_adv_mktemp (char *s, size_t l) char buf[_POSIX_PATH_MAX]; char tmp[_POSIX_PATH_MAX]; char *period; + char *r; size_t sl; struct stat sb; @@ -75,7 +76,9 @@ void mutt_adv_mktemp (char *s, size_t l) if (s[0] == '\0') { snprintf (s, l, "%s/muttXXXXXX", buf); - mktemp (s); + r = mktemp (s); + if (*r == '\0') + dprint (1, (debugfile, "%s:%d mktemp returned an empty string (errno=%d)\n", __FILE__, __LINE__, errno)); } else { @@ -87,7 +90,9 @@ void mutt_adv_mktemp (char *s, size_t l) if ((period = strrchr (tmp, '.')) != NULL) *period = 0; snprintf (s, l, "%s/%s.XXXXXX", buf, tmp); - mktemp (s); + r = mktemp (s); + if (*r == '\0') + dprint (1, (debugfile, "%s:%d mktemp returned an empty string (errno=%d)\n", __FILE__, __LINE__, errno)); if (period != NULL) { *period = '.'; diff --git a/mx.c b/mx.c index 7e527142..5d15cc59 100644 --- a/mx.c +++ b/mx.c @@ -379,6 +379,7 @@ int mx_get_magic (const char *path) } else if (st.st_size == 0) { +zero_size_file: /* hard to tell what zero-length files are, so assume the default magic */ if (DefaultMagic == M_MBOX || DefaultMagic == M_MMDF) return (DefaultMagic); @@ -389,7 +390,12 @@ int mx_get_magic (const char *path) { struct utimbuf times; - fgets (tmp, sizeof (tmp), f); + if (fgets (tmp, sizeof (tmp), f) == NULL) + { + /* This situation should not occur since we check for size==0 above.. */ + dprint(1, (debugfile, "%s:%d fgets() returned NULL. this should not happen!\n", __FILE__, __LINE__)); + goto zero_size_file; + } if (mutt_strncmp ("From ", tmp, 5) == 0) magic = M_MBOX; else if (mutt_strcmp (MMDF_SEP, tmp) == 0) diff --git a/pop.c b/pop.c index 90d95b90..e0ebd34f 100644 --- a/pop.c +++ b/pop.c @@ -67,6 +67,7 @@ static int pop_read_header (POP_DATA *pop_data, HEADER *h) long length; char buf[LONG_STRING]; char tempfile[_POSIX_PATH_MAX]; + int rv; mutt_mktemp (tempfile, sizeof (tempfile)); if (!(f = safe_fopen (tempfile, "w+"))) @@ -79,7 +80,8 @@ static int pop_read_header (POP_DATA *pop_data, HEADER *h) ret = pop_query (pop_data, buf, sizeof (buf)); if (ret == 0) { - sscanf (buf, "+OK %d %ld", &index, &length); + if ((rv = sscanf (buf, "+OK %d %ld", &index, &length)) < 2) + dprint(1, (debugfile, "%s:%d sscanf() returned %d\n", __FILE__, __LINE__, rv)); snprintf (buf, sizeof (buf), "TOP %d 0\r\n", h->refno); ret = pop_fetch_data (pop_data, buf, NULL, fetch_message, f); @@ -110,12 +112,21 @@ static int pop_read_header (POP_DATA *pop_data, HEADER *h) { rewind (f); h->env = mutt_read_rfc822_header (f, h, 0, 0); + /* + * The following code seems to be trying to alter the content length by + * removing the CR characters for the header. Note that this value is + * still incorrect, since "TOP 0" only returns the message header, so it + * only ends up accounting for the CR characters in the header. The + * correct length is set when the entire message is downloaded by + * pop_fetch_message(). + */ h->content->length = length - h->content->offset + 1; rewind (f); while (!feof (f)) { h->content->length--; - fgets (buf, sizeof (buf), f); + if (fgets (buf, sizeof (buf), f) == NULL) + ; /* EOF checked in while loop condition */ } break; } @@ -638,11 +649,13 @@ int pop_fetch_message (MESSAGE* msg, CONTEXT* ctx, int msgno) h->data = uidl; h->lines = 0; - fgets (buf, sizeof (buf), msg->fp); + if (fgets (buf, sizeof (buf), msg->fp) == NULL) + ; /* EOF checked in following while loop condition */ while (!feof (msg->fp)) { ctx->hdrs[msgno]->lines++; - fgets (buf, sizeof (buf), msg->fp); + if (fgets (buf, sizeof (buf), msg->fp) == NULL) + ; /* EOF checked in following while loop condition */ } h->content->length = ftello (msg->fp) - h->content->offset; diff --git a/query.c b/query.c index b3b8f08b..d6c8d1c9 100644 --- a/query.c +++ b/query.c @@ -93,7 +93,12 @@ static QUERY *run_query (char *s, int quiet) } if (!quiet) mutt_message _("Waiting for response..."); - fgets (msg, sizeof (msg), fp); + if (fgets (msg, sizeof (msg), fp) == NULL) + { + dprint(1, (debugfile, "%s:%d query_command produced no output (fgets() returned NULL)\n", __FILE__, __LINE__)); + return 0; + } + if ((p = strrchr (msg, '\n'))) *p = '\0'; while ((buf = mutt_read_line (buf, &buflen, fp, &dummy, 0)) != NULL) diff --git a/smime.c b/smime.c index 2ee7b4a1..7995d08b 100644 --- a/smime.c +++ b/smime.c @@ -391,8 +391,24 @@ char* smime_ask_for_key (char *prompt, char *mailbox, short public) while (!feof(index)) { numFields = fscanf (index, MUTT_FORMAT(STRING) " %x.%i " MUTT_FORMAT(STRING), fields[0], &hash, &hash_suffix, fields[2]); + /* + * ensure good values for these fields since they may not be set if + * `public` is false, and they are used below. + */ + fields[3][0] = '\0'; + fields[4][0] = '\0'; if (public) - fscanf (index, MUTT_FORMAT(STRING) " " MUTT_FORMAT(STRING) "\n", fields[3], fields[4]); + { + /* + * The original code here did not check the return value of fscanf, + * so I'm unsure whether the entire line should be ignored upon + * error. Just log it for now. + */ + int rv; + rv = fscanf (index, MUTT_FORMAT(STRING) " " MUTT_FORMAT(STRING) "\n", fields[3], fields[4]); + if (rv < 2) + dprint (1, (debugfile, "%s:%d fscanf() returned %d\n", __FILE__, __LINE__, errno)); + } /* 0=email 1=name 2=nick 3=intermediate 4=trust */ if (numFields < 2) continue; diff --git a/system.c b/system.c index 5b458654..79766e55 100644 --- a/system.c +++ b/system.c @@ -92,7 +92,8 @@ int _mutt_system (const char *cmd, int flags) close (1); close (2); #endif - chdir ("/"); + if (chdir ("/") == -1) + _exit (127); act.sa_handler = SIG_DFL; sigaction (SIGCHLD, &act, NULL); break; -- 2.40.0