tidying: coverity issues

10 files changed, 310 insertions, 281 deletions

diff --git a/src/src/acl.c b/src/src/acl.c
index f17e28488..236cfb7bd 100644
--- a/src/src/acl.c
+++ b/src/src/acl.c
@@ -1076,9 +1076,9 @@ while (*hstring == '\n') hstring++, hlen--;
 
 /* An empty string does nothing; ensure exactly one final newline. */
 if (hlen <= 0) return;
-if (hstring[--hlen] != '\n')
+if (hstring[--hlen] != '\n')		/* no newline */
   q = string_sprintf("%s\n", hstring);
-else if (hstring[hlen-1] == '\n')
+else if (hstring[hlen-1] == '\n')	/* double newline */
   {
   uschar * s = string_copy(hstring);
   while(s[--hlen] == '\n')
@@ -1101,7 +1101,7 @@ for (p = q; *p != 0; )
 
   for (;;)
     {
-    q = Ustrchr(q, '\n');
+    q = Ustrchr(q, '\n');		/* we know there was a newline */
     if (*(++q) != ' ' && *q != '\t') break;
     }
 
@@ -2376,17 +2376,13 @@ rate measurement as opposed to rate limiting. */
 
 sender_rate_limit = string_nextinlist(&arg, &sep, NULL, 0);
 if (sender_rate_limit == NULL)
-  {
-  limit = -1.0;
-  ss = NULL;	/* compiler quietening */
-  }
-else
-  {
-  limit = Ustrtod(sender_rate_limit, &ss);
-  if (tolower(*ss) == 'k') { limit *= 1024.0; ss++; }
-  else if (tolower(*ss) == 'm') { limit *= 1024.0*1024.0; ss++; }
-  else if (tolower(*ss) == 'g') { limit *= 1024.0*1024.0*1024.0; ss++; }
-  }
+  return ratelimit_error(log_msgptr, "sender rate limit not set");
+
+limit = Ustrtod(sender_rate_limit, &ss);
+if      (tolower(*ss) == 'k') { limit *= 1024.0; ss++; }
+else if (tolower(*ss) == 'm') { limit *= 1024.0*1024.0; ss++; }
+else if (tolower(*ss) == 'g') { limit *= 1024.0*1024.0*1024.0; ss++; }
+
 if (limit < 0.0 || *ss != '\0')
   return ratelimit_error(log_msgptr,
     "\"%s\" is not a positive number", sender_rate_limit);
diff --git a/src/src/auths/dovecot.c b/src/src/auths/dovecot.c
index c89411af8..9641beff4 100644
--- a/src/src/auths/dovecot.c
+++ b/src/src/auths/dovecot.c
@@ -228,244 +228,270 @@ return s;
 *              Server entry point                *
 *************************************************/
 
-int auth_dovecot_server(auth_instance *ablock, uschar *data)
+int
+auth_dovecot_server(auth_instance * ablock, uschar * data)
 {
-       auth_dovecot_options_block *ob =
-               (auth_dovecot_options_block *)(ablock->options_block);
-       struct sockaddr_un sa;
-       uschar buffer[DOVECOT_AUTH_MAXLINELEN];
-       uschar *args[DOVECOT_AUTH_MAXFIELDCOUNT];
-       uschar *auth_command;
-       uschar *auth_extra_data = US"";
-       uschar *p;
-       int nargs, tmp;
-       int crequid = 1, cont = 1, fd, ret = DEFER;
-       BOOL found = FALSE, have_mech_line = FALSE;
-
-       HDEBUG(D_auth) debug_printf("dovecot authentication\n");
-
-       memset(&sa, 0, sizeof(sa));
-       sa.sun_family = AF_UNIX;
-
-       /* This was the original code here: it is nonsense because strncpy()
-       does not return an integer. I have converted this to use the function
-       that formats and checks length. PH */
-
-       /*
-       if (strncpy(sa.sun_path, ob->server_socket, sizeof(sa.sun_path)) < 0) {
-       */
-
-       if (!string_format(US sa.sun_path, sizeof(sa.sun_path), "%s",
-                          ob->server_socket)) {
-               auth_defer_msg = US"authentication socket path too long";
-               return DEFER;
-       }
-
-       auth_defer_msg = US"authentication socket connection error";
-
-       fd = socket(PF_UNIX, SOCK_STREAM, 0);
-       if (fd < 0)
-               return DEFER;
-
-       if (connect(fd, (struct sockaddr *) &sa, sizeof(sa)) < 0)
-               goto out;
+auth_dovecot_options_block *ob =
+       (auth_dovecot_options_block *) ablock->options_block;
+struct sockaddr_un sa;
+uschar buffer[DOVECOT_AUTH_MAXLINELEN];
+uschar *args[DOVECOT_AUTH_MAXFIELDCOUNT];
+uschar *auth_command;
+uschar *auth_extra_data = US"";
+uschar *p;
+int nargs, tmp;
+int crequid = 1, cont = 1, fd, ret = DEFER;
+BOOL found = FALSE, have_mech_line = FALSE;
+
+HDEBUG(D_auth) debug_printf("dovecot authentication\n");
+
+if (!data)
+  {
+  ret = FAIL;
+  goto out;
+  }
 
-       auth_defer_msg = US"authentication socket protocol error";
+memset(&sa, 0, sizeof(sa));
+sa.sun_family = AF_UNIX;
 
-       socket_buffer_left = 0;  /* Global, used to read more than a line but return by line */
-       while (cont) {
-               if (dc_gets(buffer, sizeof(buffer), fd) == NULL)
-                       OUT("authentication socket read error or premature eof");
-               p = buffer + Ustrlen(buffer) - 1;
-               if (*p != '\n') {
-                       OUT("authentication socket protocol line too long");
-               }
-               *p = '\0';
-               HDEBUG(D_auth) debug_printf("received: %s\n", buffer);
-               nargs = strcut(buffer, args, sizeof(args) / sizeof(args[0]));
-               /* HDEBUG(D_auth) debug_strcut(args, nargs, sizeof(args) / sizeof(args[0])); */
-
-               /* Code below rewritten by Kirill Miazine (km@krot.org). Only check commands that
-                  Exim will need. Original code also failed if Dovecot server sent unknown
-                  command. E.g. COOKIE in version 1.1 of the protocol would cause troubles. */
-               /* pdp: note that CUID is a per-connection identifier sent by the server,
-                  which increments at server discretion.
-                  By contrast, the "id" field of the protocol is a connection-specific request
-                  identifier, which needs to be unique per request from the client and is not
-                  connected to the CUID value, so we ignore CUID from server.  It's purely for
-                  diagnostics. */
-               if (Ustrcmp(args[0], US"VERSION") == 0) {
-                       CHECK_COMMAND("VERSION", 2, 2);
-                       if (Uatoi(args[1]) != VERSION_MAJOR)
-                               OUT("authentication socket protocol version mismatch");
-               } else if (Ustrcmp(args[0], US"MECH") == 0) {
-                       CHECK_COMMAND("MECH", 1, INT_MAX);
-                       have_mech_line = TRUE;
-                       if (strcmpic(US args[1], ablock->public_name) == 0)
-                               found = TRUE;
-               } else if (Ustrcmp(args[0], US"SPID") == 0) {
-                       /* Unfortunately the auth protocol handshake wasn't designed well
-                          to differentiate between auth-client/userdb/master. auth-userdb
-                          and auth-master send VERSION + SPID lines only and nothing
-                          afterwards, while auth-client sends VERSION + MECH + SPID +
-                          CUID + more. The simplest way that we can determine if we've
-                          connected to the correct socket is to see if MECH line exists or
-                          not (alternatively we'd have to have a small timeout after SPID
-                          to see if CUID is sent or not). */
-                       if (!have_mech_line)
-                               OUT("authentication socket type mismatch (connected to auth-master instead of auth-client)");
-               } else if (Ustrcmp(args[0], US"DONE") == 0) {
-                       CHECK_COMMAND("DONE", 0, 0);
-                       cont = 0;
-               }
-       }
+/* This was the original code here: it is nonsense because strncpy()
+does not return an integer. I have converted this to use the function
+that formats and checks length. PH */
 
-       if (!found) {
-               auth_defer_msg = string_sprintf("Dovecot did not advertise mechanism \"%s\" to us", ablock->public_name);
-               goto out;
-       }
+/*
+if (strncpy(sa.sun_path, ob->server_socket, sizeof(sa.sun_path)) < 0) {
+}
+*/
 
-       /* Added by PH: data must not contain tab (as it is
-       b64 it shouldn't, but check for safety). */
+if (!string_format(US sa.sun_path, sizeof(sa.sun_path), "%s",
+		  ob->server_socket))
+  {
+  auth_defer_msg = US"authentication socket path too long";
+  return DEFER;
+  }
 
-       if (Ustrchr(data, '\t') != NULL) {
-               ret = FAIL;
-               goto out;
-       }
+auth_defer_msg = US"authentication socket connection error";
 
-       /* Added by PH: extra fields when TLS is in use or if the TCP/IP
-       connection is local. */
+if ((fd = socket(PF_UNIX, SOCK_STREAM, 0)) < 0)
+  return DEFER;
 
-       if (tls_in.cipher != NULL)
-               auth_extra_data = string_sprintf("secured\t%s%s",
-                   tls_in.certificate_verified? "valid-client-cert" : "",
-                   tls_in.certificate_verified? "\t" : "");
-       else if (interface_address != NULL &&
-                Ustrcmp(sender_host_address, interface_address) == 0)
-               auth_extra_data = US"secured\t";
+if (connect(fd, (struct sockaddr *) &sa, sizeof(sa)) < 0)
+  goto out;
 
+auth_defer_msg = US"authentication socket protocol error";
 
-/****************************************************************************
-   The code below was the original code here. It didn't work. A reading of the
-   file auth-protocol.txt.gz that came with Dovecot 1.0_beta8 indicated that
-   this was not right. Maybe something changed. I changed it to move the
-   service indication into the AUTH command, and it seems to be better. PH
-
-       fprintf(f, "VERSION\t%d\t%d\r\nSERVICE\tSMTP\r\nCPID\t%d\r\n"
-               "AUTH\t%d\t%s\trip=%s\tlip=%s\tresp=%s\r\n",
-               VERSION_MAJOR, VERSION_MINOR, getpid(), cuid,
-               ablock->public_name, sender_host_address, interface_address,
-               data ? (char *) data : "");
-
-   Subsequently, the command was modified to add "secured" and "valid-client-
-   cert" when relevant.
-****************************************************************************/
+socket_buffer_left = 0;  /* Global, used to read more than a line but return by line */
+while (cont)
+  {
+  if (dc_gets(buffer, sizeof(buffer), fd) == NULL)
+    OUT("authentication socket read error or premature eof");
+  p = buffer + Ustrlen(buffer) - 1;
+  if (*p != '\n')
+    OUT("authentication socket protocol line too long");
+
+  *p = '\0';
+  HDEBUG(D_auth) debug_printf("received: %s\n", buffer);
+
+  nargs = strcut(buffer, args, sizeof(args) / sizeof(args[0]));
+
+  /* HDEBUG(D_auth) debug_strcut(args, nargs, sizeof(args) / sizeof(args[0])); */
+
+  /* Code below rewritten by Kirill Miazine (km@krot.org). Only check commands that
+    Exim will need. Original code also failed if Dovecot server sent unknown
+    command. E.g. COOKIE in version 1.1 of the protocol would cause troubles. */
+  /* pdp: note that CUID is a per-connection identifier sent by the server,
+    which increments at server discretion.
+    By contrast, the "id" field of the protocol is a connection-specific request
+    identifier, which needs to be unique per request from the client and is not
+    connected to the CUID value, so we ignore CUID from server.  It's purely for
+    diagnostics. */
+
+  if (Ustrcmp(args[0], US"VERSION") == 0)
+    {
+    CHECK_COMMAND("VERSION", 2, 2);
+    if (Uatoi(args[1]) != VERSION_MAJOR)
+      OUT("authentication socket protocol version mismatch");
+    }
+  else if (Ustrcmp(args[0], US"MECH") == 0)
+    {
+    CHECK_COMMAND("MECH", 1, INT_MAX);
+    have_mech_line = TRUE;
+    if (strcmpic(US args[1], ablock->public_name) == 0)
+      found = TRUE;
+    }
+  else if (Ustrcmp(args[0], US"SPID") == 0)
+    {
+    /* Unfortunately the auth protocol handshake wasn't designed well
+    to differentiate between auth-client/userdb/master. auth-userdb
+    and auth-master send VERSION + SPID lines only and nothing
+    afterwards, while auth-client sends VERSION + MECH + SPID +
+    CUID + more. The simplest way that we can determine if we've
+    connected to the correct socket is to see if MECH line exists or
+    not (alternatively we'd have to have a small timeout after SPID
+    to see if CUID is sent or not). */
+
+    if (!have_mech_line)
+      OUT("authentication socket type mismatch"
+	" (connected to auth-master instead of auth-client)");
+    }
+  else if (Ustrcmp(args[0], US"DONE") == 0)
+    {
+    CHECK_COMMAND("DONE", 0, 0);
+    cont = 0;
+    }
+  }
 
-       auth_command = string_sprintf("VERSION\t%d\t%d\nCPID\t%d\n"
-               "AUTH\t%d\t%s\tservice=smtp\t%srip=%s\tlip=%s\tnologin\tresp=%s\n",
-               VERSION_MAJOR, VERSION_MINOR, getpid(), crequid,
-               ablock->public_name, auth_extra_data, sender_host_address,
-               interface_address, data ? (char *) data : "");
+if (!found)
+  {
+  auth_defer_msg = string_sprintf(
+    "Dovecot did not advertise mechanism \"%s\" to us", ablock->public_name);
+  goto out;
+  }
 
-       if (write(fd, auth_command, Ustrlen(auth_command)) < 0)
-              HDEBUG(D_auth) debug_printf("error sending auth_command: %s\n",
-                strerror(errno));
+/* Added by PH: data must not contain tab (as it is
+b64 it shouldn't, but check for safety). */
 
-       HDEBUG(D_auth) debug_printf("sent: %s", auth_command);
+if (Ustrchr(data, '\t') != NULL)
+  {
+  ret = FAIL;
+  goto out;
+  }
 
-       while (1) {
-               uschar *temp;
-               uschar *auth_id_pre = NULL;
-               int i;
+/* Added by PH: extra fields when TLS is in use or if the TCP/IP
+connection is local. */
 
-               if (dc_gets(buffer, sizeof(buffer), fd) == NULL) {
-                       auth_defer_msg = US"authentication socket read error or premature eof";
-                       goto out;
-               }
+if (tls_in.cipher != NULL)
+  auth_extra_data = string_sprintf("secured\t%s%s",
+     tls_in.certificate_verified? "valid-client-cert" : "",
+     tls_in.certificate_verified? "\t" : "");
 
-               buffer[Ustrlen(buffer) - 1] = 0;
-               HDEBUG(D_auth) debug_printf("received: %s\n", buffer);
-               nargs = strcut(buffer, args, sizeof(args) / sizeof(args[0]));
+else if (  interface_address != NULL
+        && Ustrcmp(sender_host_address, interface_address) == 0)
+  auth_extra_data = US"secured\t";
 
-               if (Uatoi(args[1]) != crequid)
-                       OUT("authentication socket connection id mismatch");
 
-               switch (toupper(*args[0])) {
-               case 'C':
-                       CHECK_COMMAND("CONT", 1, 2);
+/****************************************************************************
+The code below was the original code here. It didn't work. A reading of the
+file auth-protocol.txt.gz that came with Dovecot 1.0_beta8 indicated that
+this was not right. Maybe something changed. I changed it to move the
+service indication into the AUTH command, and it seems to be better. PH
+
+fprintf(f, "VERSION\t%d\t%d\r\nSERVICE\tSMTP\r\nCPID\t%d\r\n"
+       "AUTH\t%d\t%s\trip=%s\tlip=%s\tresp=%s\r\n",
+       VERSION_MAJOR, VERSION_MINOR, getpid(), cuid,
+       ablock->public_name, sender_host_address, interface_address,
+       data ? (char *) data : "");
+
+Subsequently, the command was modified to add "secured" and "valid-client-
+cert" when relevant.
+****************************************************************************/
 
-                       tmp = auth_get_no64_data(&data, US args[2]);
-                       if (tmp != OK) {
-                               ret = tmp;
-                               goto out;
-                       }
+auth_command = string_sprintf("VERSION\t%d\t%d\nCPID\t%d\n"
+       "AUTH\t%d\t%s\tservice=smtp\t%srip=%s\tlip=%s\tnologin\tresp=%s\n",
+       VERSION_MAJOR, VERSION_MINOR, getpid(), crequid,
+       ablock->public_name, auth_extra_data, sender_host_address,
+       interface_address, data);
 
-                       /* Added by PH: data must not contain tab (as it is
-                       b64 it shouldn't, but check for safety). */
+if (write(fd, auth_command, Ustrlen(auth_command)) < 0)
+  HDEBUG(D_auth) debug_printf("error sending auth_command: %s\n",
+    strerror(errno));
 
-                       if (Ustrchr(data, '\t') != NULL) {
-                               ret = FAIL;
-                               goto out;
-                       }
+HDEBUG(D_auth) debug_printf("sent: %s", auth_command);
 
-                       temp = string_sprintf("CONT\t%d\t%s\n", crequid, data);
-                       if (write(fd, temp, Ustrlen(temp)) < 0)
-                               OUT("authentication socket write error");
-                       break;
-
-               case 'F':
-                       CHECK_COMMAND("FAIL", 1, -1);
-
-                       for (i=2; (i<nargs) && (auth_id_pre == NULL); i++)
-                       {
-                               if ( Ustrncmp(args[i], US"user=", 5) == 0 )
-                               {
-                                       auth_id_pre = args[i]+5;
-                                       expand_nstring[1] = auth_vars[0] =
-                                               string_copy(auth_id_pre); /* PH */
-                                       expand_nlength[1] = Ustrlen(auth_id_pre);
-                                       expand_nmax = 1;
-                               }
-                       }
+while (1)
+  {
+  uschar *temp;
+  uschar *auth_id_pre = NULL;
+  int i;
 
-                       ret = FAIL;
-                       goto out;
-
-               case 'O':
-                       CHECK_COMMAND("OK", 2, -1);
-
-                       /*
-                        * Search for the "user=$USER" string in the args array
-                        * and return the proper value.
-                        */
-                       for (i=2; (i<nargs) && (auth_id_pre == NULL); i++)
-                       {
-                               if ( Ustrncmp(args[i], US"user=", 5) == 0 )
-                               {
-                                       auth_id_pre = args[i]+5;
-                                       expand_nstring[1] = auth_vars[0] =
-                                               string_copy(auth_id_pre); /* PH */
-                                       expand_nlength[1] = Ustrlen(auth_id_pre);
-                                       expand_nmax = 1;
-                               }
-                       }
+  if (dc_gets(buffer, sizeof(buffer), fd) == NULL)
+    {
+    auth_defer_msg = US"authentication socket read error or premature eof";
+    goto out;
+    }
 
-                       if (auth_id_pre == NULL)
-                               OUT("authentication socket protocol error, username missing");
+  buffer[Ustrlen(buffer) - 1] = 0;
+  HDEBUG(D_auth) debug_printf("received: %s\n", buffer);
+  nargs = strcut(buffer, args, sizeof(args) / sizeof(args[0]));
 
-                       ret = OK;
-                       /* fallthrough */
+  if (Uatoi(args[1]) != crequid)
+    OUT("authentication socket connection id mismatch");
 
-               default:
-                       goto out;
-               }
-       }
+  switch (toupper(*args[0]))
+    {
+    case 'C':
+      CHECK_COMMAND("CONT", 1, 2);
+
+      if ((tmp = auth_get_no64_data(&data, US args[2])) != OK)
+	{
+	ret = tmp;
+	goto out;
+	}
+
+      /* Added by PH: data must not contain tab (as it is
+      b64 it shouldn't, but check for safety). */
+
+      if (Ustrchr(data, '\t') != NULL)
+        {
+	ret = FAIL;
+	goto out;
+	}
+
+      temp = string_sprintf("CONT\t%d\t%s\n", crequid, data);
+      if (write(fd, temp, Ustrlen(temp)) < 0)
+	OUT("authentication socket write error");
+      break;
+
+    case 'F':
+      CHECK_COMMAND("FAIL", 1, -1);
+
+      for (i=2; (i<nargs) && (auth_id_pre == NULL); i++)
+	{
+	if ( Ustrncmp(args[i], US"user=", 5) == 0 )
+	  {
+	  auth_id_pre = args[i]+5;
+	  expand_nstring[1] = auth_vars[0] = string_copy(auth_id_pre); /* PH */
+	  expand_nlength[1] = Ustrlen(auth_id_pre);
+	  expand_nmax = 1;
+	  }
+	}
+
+      ret = FAIL;
+      goto out;
+
+    case 'O':
+      CHECK_COMMAND("OK", 2, -1);
+
+      /* Search for the "user=$USER" string in the args array
+      and return the proper value.  */
+
+      for (i=2; (i<nargs) && (auth_id_pre == NULL); i++)
+	{
+	if ( Ustrncmp(args[i], US"user=", 5) == 0 )
+	  {
+	  auth_id_pre = args[i]+5;
+	  expand_nstring[1] = auth_vars[0] = string_copy(auth_id_pre); /* PH */
+	  expand_nlength[1] = Ustrlen(auth_id_pre);
+	  expand_nmax = 1;
+	  }
+	}
+
+      if (auth_id_pre == NULL)
+        OUT("authentication socket protocol error, username missing");
+
+      ret = OK;
+      /* fallthrough */
+
+    default:
+      goto out;
+    }
+  }
 
 out:
-       /* close the socket used by dovecot */
-       if (fd >= 0)
-              close(fd);
+/* close the socket used by dovecot */
+if (fd >= 0)
+  close(fd);
 
-       /* Expand server_condition as an authorization check */
-       return (ret == OK)? auth_check_serv_cond(ablock) : ret;
+/* Expand server_condition as an authorization check */
+return ret == OK ? auth_check_serv_cond(ablock) : ret;
 }
diff --git a/src/src/daemon.c b/src/src/daemon.c
index ac09e2a64..6c1919304 100644
--- a/src/src/daemon.c
+++ b/src/src/daemon.c
@@ -927,7 +927,7 @@ if (inetd_wait_mode)
   int on = 1;
 
   listen_socket_count = 1;
-  listen_sockets = store_get(sizeof(int *));
+  listen_sockets = store_get(sizeof(int));
   (void) close(3);
   if (dup2(0, 3) == -1)
     log_write(0, LOG_MAIN|LOG_PANIC_DIE,
@@ -1277,7 +1277,7 @@ if (daemon_listen && !inetd_wait_mode)
 
   for (ipa = addresses; ipa != NULL; ipa = ipa->next)
     listen_socket_count++;
-  listen_sockets = store_get(sizeof(int *) * listen_socket_count);
+  listen_sockets = store_get(sizeof(int) * listen_socket_count);
 
   } /* daemon_listen but not inetd_wait_mode */
 
@@ -1382,8 +1382,7 @@ if (daemon_listen && !inetd_wait_mode)
       wildcard = ipa->address[0] == 0;
       }
 
-    listen_sockets[sk] = ip_socket(SOCK_STREAM, af);
-    if (listen_sockets[sk] < 0)
+    if ((listen_sockets[sk] = ip_socket(SOCK_STREAM, af)) < 0)
       {
       if (check_special_case(0, addresses, ipa, FALSE))
         {
diff --git a/src/src/dcc.c b/src/src/dcc.c
index c374cf91c..5d5c0b022 100644
--- a/src/src/dcc.c
+++ b/src/src/dcc.c
@@ -434,11 +434,11 @@ dcc_process(uschar **listptr)
             }
           }
           else {
-          /* We're on the first line but not on the first character,
-           * there must be something wrong. */
-            DEBUG(D_acl)
-              debug_printf("DCC: Line = %d but i = %d != 0  character is %c - This is wrong!\n", line, i, recvbuf[i]);
-              log_write(0,LOG_MAIN,"Wrong header from DCC, output is %s\n", recvbuf);
+            /* We're on the first line but not on the first character,
+             * there must be something wrong. */
+            DEBUG(D_acl) debug_printf("DCC: Line = %d but i = %d != 0"
+		"  character is %c - This is wrong!\n", line, i, recvbuf[i]);
+            log_write(0,LOG_MAIN,"Wrong header from DCC, output is %s\n", recvbuf);
           }
         }
         else if(line == 2) {
@@ -456,8 +456,8 @@ dcc_process(uschar **listptr)
             k++;
           }
           else {
-            DEBUG(D_acl)
-              debug_printf("DCC: We got more output than we can store in the X-DCC header. Truncating at 120 characters.\n");
+            DEBUG(D_acl) debug_printf("DCC: We got more output than we can store"
+		" in the X-DCC header. Truncating at 120 characters.\n");
           }
         }
         else {
diff --git a/src/src/drtables.c b/src/src/drtables.c
index d78b766a5..d427f3215 100644
--- a/src/src/drtables.c
+++ b/src/src/drtables.c
@@ -415,37 +415,42 @@ struct lookupmodulestr
 
 static struct lookupmodulestr *lookupmodules = NULL;
 
-static void addlookupmodule(void *dl, struct lookup_module_info *info)
+static void
+addlookupmodule(void *dl, struct lookup_module_info *info)
 {
-  struct lookupmodulestr *p = store_malloc(sizeof(struct lookupmodulestr));
-  p->dl = dl;
-  p->info = info;
-  p->next = lookupmodules;
-  lookupmodules = p;
-  lookup_list_count += info->lookupcount;
+struct lookupmodulestr *p = store_malloc(sizeof(struct lookupmodulestr));
+
+p->dl = dl;
+p->info = info;
+p->next = lookupmodules;
+lookupmodules = p;
+lookup_list_count += info->lookupcount;
 }
 
 /* only valid after lookup_list and lookup_list_count are assigned */
-static void add_lookup_to_list(lookup_info *info)
+static void
+add_lookup_to_list(lookup_info *info)
 {
-  /* need to add the lookup to lookup_list, sorted */
-  int pos = 0;
-
-  /* strategy is to go through the list until we find
-   * either an empty spot or a name that is higher.
-   * this can't fail because we have enough space. */
-  while (lookup_list[pos]
-      && (Ustrcmp(lookup_list[pos]->name, info->name) <= 0)) {
-    pos++;
-  }
-  if (lookup_list[pos]) {
-    /* need to insert it, so move all the other items up
-     * (last slot is still empty, of course) */
-    memmove(&lookup_list[pos+1],
-            &lookup_list[pos],
-            sizeof(lookup_info **) * (lookup_list_count-pos-1));
+/* need to add the lookup to lookup_list, sorted */
+int pos = 0;
+
+/* strategy is to go through the list until we find
+either an empty spot or a name that is higher.
+this can't fail because we have enough space. */
+
+while (lookup_list[pos] && (Ustrcmp(lookup_list[pos]->name, info->name) <= 0))
+  pos++;
+
+if (lookup_list[pos])
+  {
+  /* need to insert it, so move all the other items up
+  (last slot is still empty, of course) */
+
+  memmove(&lookup_list[pos+1],
+	  &lookup_list[pos],
+	  sizeof(lookup_info *) * (lookup_list_count-pos-1));
   }
-  lookup_list[pos] = info;
+lookup_list[pos] = info;
 }
 
 
@@ -508,7 +513,9 @@ extern lookup_module_info testdb_lookup_module_info;
 extern lookup_module_info whoson_lookup_module_info;
 #endif
 
-void init_lookup_list(void)
+
+void
+init_lookup_list(void)
 {
 #ifdef LOOKUP_MODULE_DIR
   DIR *dd;
diff --git a/src/src/host.c b/src/src/host.c
index b65168c68..8e71aec5f 100644
--- a/src/src/host.c
+++ b/src/src/host.c
@@ -257,7 +257,7 @@ else
       count++;
 
   yield = store_get(sizeof(struct hostent));
-  alist = store_get((count + 1) * sizeof(char **));
+  alist = store_get((count + 1) * sizeof(char *));
   adds  = store_get(count *alen);
 
   yield->h_name = CS name;
diff --git a/src/src/rda.c b/src/src/rda.c
index 2afd6dc8a..cf663a5ca 100644
--- a/src/src/rda.c
+++ b/src/src/rda.c
@@ -911,7 +911,7 @@ if (yield == FF_DELIVERED || yield == FF_NOTDELIVERED ||
 
     if (i > 0)
       {
-      addr->pipe_expandn = store_get((i+1) * sizeof(uschar **));
+      addr->pipe_expandn = store_get((i+1) * sizeof(uschar *));
       addr->pipe_expandn[i] = NULL;
       while (--i >= 0) addr->pipe_expandn[i] = expandn[i];
       }
diff --git a/src/src/sieve.c b/src/src/sieve.c
index 8edb0b8b7..bda482fd1 100644
--- a/src/src/sieve.c
+++ b/src/src/sieve.c
@@ -283,7 +283,7 @@ for (pass=0; pass<=1; ++pass)
         }
       else
         *new++='\n';
-        line=0;
+        line=0;		/*XXX jgh: questionabale indent; probable BUG */
       ++start;
       }
     else
diff --git a/src/src/string.c b/src/src/string.c
index d74787213..4ef6a3773 100644
--- a/src/src/string.c
+++ b/src/src/string.c
@@ -1087,8 +1087,8 @@ Returns:   pointer to the start of the string, changed if copied for expansion.
            because string_cat() is often called multiple times to build up a
            string - there's no point adding the NUL till the end.
 
-coverity[+alloc]
 */
+/* coverity[+alloc] */
 
 uschar *
 string_cat(uschar *string, int *size, int *ptr, const uschar *s, int count)
@@ -1141,7 +1141,7 @@ common use is a null string and zero size and pointer, on first use for a
 string being built. The "if" above then allocates, but Coverity assume that
 the "if" might not happen and whines for a null-deref done by the memcpy(). */
 
-/* coverity[var_deref_op] */
+/* coverity[deref_parm_field_in_call] */
 memcpy(string + p, s, count);
 *ptr = p + count;
 return string;
diff --git a/src/src/tls-openssl.c b/src/src/tls-openssl.c
index ca687a632..85ba2e5bd 100644
--- a/src/src/tls-openssl.c
+++ b/src/src/tls-openssl.c
@@ -2362,27 +2362,28 @@ while (left > 0)
   switch (error)
     {
     case SSL_ERROR_SSL:
-    ERR_error_string(ERR_get_error(), ssl_errstring);
-    log_write(0, LOG_MAIN, "TLS error (SSL_write): %s", ssl_errstring);
-    return -1;
+      ERR_error_string(ERR_get_error(), ssl_errstring);
+      log_write(0, LOG_MAIN, "TLS error (SSL_write): %s", ssl_errstring);
+      return -1;
 
     case SSL_ERROR_NONE:
-    left -= outbytes;
-    buff += outbytes;
-    break;
+      left -= outbytes;
+      buff += outbytes;
+      break;
 
     case SSL_ERROR_ZERO_RETURN:
-    log_write(0, LOG_MAIN, "SSL channel closed on write");
-    return -1;
+      log_write(0, LOG_MAIN, "SSL channel closed on write");
+      return -1;
 
     case SSL_ERROR_SYSCALL:
-    log_write(0, LOG_MAIN, "SSL_write: (from %s) syscall: %s",
-      sender_fullhost ? sender_fullhost : US"<unknown>",
-      strerror(errno));
+      log_write(0, LOG_MAIN, "SSL_write: (from %s) syscall: %s",
+	sender_fullhost ? sender_fullhost : US"<unknown>",
+	strerror(errno));
+      return -1;
 
     default:
-    log_write(0, LOG_MAIN, "SSL_write error %d", error);
-    return -1;
+      log_write(0, LOG_MAIN, "SSL_write error %d", error);
+      return -1;
     }
   }
 return len;
@@ -2819,6 +2820,7 @@ for (s=option_spec; *s != '\0'; /**/)
   keep_c = *end;
   *end = '\0';
   item_parsed = tls_openssl_one_option_parse(s, &item);
+  *end = keep_c;
   if (!item_parsed)
     {
     DEBUG(D_tls) debug_printf("openssl option setting unrecognised: \"%s\"\n", s);
@@ -2830,7 +2832,6 @@ for (s=option_spec; *s != '\0'; /**/)
     result |= item;
   else
     result &= ~item;
-  *end = keep_c;
   s = end;
   }