Compilation warnings shushing

With this patch, in clang 3.4.1 we get no compilation complaints if
Local/Makefile contains:

    CC=clang
    CFLAGS+=-Wno-dangling-else -Wno-logical-op-parentheses

* In hash.c, for the OpenSSL case, use assert() to guard the paths which
  can't happen, instead of just assuming that the calling code never has
  a mistake
* Fix some signed/unsigned issues
* Be explicit about some ignored return values
* Some parens around bit-twiddling
* Use our os_getcwd with its extra guards in one place where getcwd was
  called
* FreeBSD: use system iconv, safely, always

(cherry picked from commit 845a3ced80964f562872aba841099adbc8933b40)
Signed-off-by: Phil Pennock <pdp@exim.org>

9 files changed, 38 insertions, 13 deletions

diff --git a/src/OS/os.h-FreeBSD b/src/OS/os.h-FreeBSD
index ba4889fec..a67ca13b7 100644
--- a/src/OS/os.h-FreeBSD
+++ b/src/OS/os.h-FreeBSD
@@ -10,7 +10,21 @@
 
 typedef struct flock flock_t;
 
-/* default is non-const */
-#define ICONV_ARG2_TYPE const char **
+/* iconv arg2 type: libiconv in Ports uses "const char* * inbuf" and was
+ * traditionally the only approach available.  The iconv functionality
+ * in libc is "char ** restrict src".
+ *
+ * <https://www.freebsd.org/doc/en/books/porters-handbook/using-iconv.html>
+ * says that libc has iconv since 2013, in 10-CURRENT.  FreeBSD man-pages
+ * shows it included in 10.0-RELEASE.  Writing this in 2017, 10.3 is the
+ * oldest supported release, so we should assume non-libiconv by default.
+ *
+ * Thus we no longer override iconv.
+ *
+ * However, if libiconv is installed, and anything adds /usr/local/include
+ * to include-path (likely) then we'll get that.  So define a variable
+ * which makes the libiconv try to not interfere with OS iconv.
+ */
+#define LIBICONV_PLUG
 
 /* End */
diff --git a/src/src/EDITME b/src/src/EDITME
index 15932691e..df74aacde 100644
--- a/src/src/EDITME
+++ b/src/src/EDITME
@@ -689,6 +689,13 @@ HEADERS_CHARSET="ISO-8859-1"
 #
 # but of course there may need to be other things in CFLAGS and EXTRALIBS_EXIM
 # as well.
+#
+# nb: FreeBSD as of 4.89 defines LIBICONV_PLUG to pick up the system iconv
+# more reliably.  If you explicitly want the libiconv Port then as well
+# as adding -liconv you'll want to unset LIBICONV_PLUG.  If you actually need
+# this, let us know, but for now the Exim Maintainers are assuming that this
+# is uncommon and so you'll need to edit OS/os.h-FreeBSD yourself to remove
+# the define.
 
 
 #------------------------------------------------------------------------------
diff --git a/src/src/deliver.c b/src/src/deliver.c
index 85208457f..c97874a2b 100644
--- a/src/src/deliver.c
+++ b/src/src/deliver.c
@@ -4344,7 +4344,7 @@ for (delivery_count = 0; addr_remote; delivery_count++)
 	  )  )
        && (  !multi_domain
 	  || (  (
-		!tp->expand_multi_domain || (deliver_set_expansions(next), 1),
+		(void)(!tp->expand_multi_domain || ((void)deliver_set_expansions(next), 1)),
 	        exp_bool(addr,
 		    US"transport", next->transport->name, D_transport,
 		    US"multi_domain", next->transport->multi_domain,
diff --git a/src/src/exim.c b/src/src/exim.c
index 1f450c533..a6a1ea82c 100644
--- a/src/src/exim.c
+++ b/src/src/exim.c
@@ -2294,7 +2294,7 @@ for (i = 1; i < argc; i++)
       #ifdef ALT_CONFIG_PREFIX
       int sep = 0;
       int len = Ustrlen(ALT_CONFIG_PREFIX);
-      uschar *list = argrest;
+      const uschar *list = argrest;
       uschar *filename;
       while((filename = string_nextinlist(&list, &sep, big_buffer,
              big_buffer_size)) != NULL)
diff --git a/src/src/hash.c b/src/src/hash.c
index 059e6d9bb..7590d55b7 100644
--- a/src/src/hash.c
+++ b/src/src/hash.c
@@ -25,7 +25,7 @@ typedef struct sha1 {
 sha1;
 #endif	/*STAND_ALONE*/
 
-
+#include <assert.h>
 
 /******************************************************************************/
 #ifdef SHA_OPENSSL
@@ -50,6 +50,9 @@ switch (h->method)
   {
   case HASH_SHA1:   SHA1_Update  (&h->u.sha1, data, len); break;
   case HASH_SHA256: SHA256_Update(&h->u.sha2, data, len); break;
+  /* should be blocked by init not handling these, but be explicit to
+   * guard against accidents later (and hush up clang -Wswitch) */
+  default: assert(0);
   }
 }
 
@@ -62,6 +65,7 @@ switch (h->method)
   {
   case HASH_SHA1:   SHA1_Final  (b->data, &h->u.sha1); break;
   case HASH_SHA256: SHA256_Final(b->data, &h->u.sha2); break;
+  default: assert(0);
   }
 }
 
diff --git a/src/src/local_scan.h b/src/src/local_scan.h
index bca14bcaf..bc4fc8e25 100644
--- a/src/src/local_scan.h
+++ b/src/src/local_scan.h
@@ -115,7 +115,7 @@ typedef struct header_line {
 /* Entries in lists options are in this form. */
 
 typedef struct {
-  const char   *name;
+  const char   *name; /* should have been uschar but too late now */
   int           type;
   void         *value;
 } optionlist;
diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c
index 6e76b8e9f..4c93de70d 100644
--- a/src/src/pdkim/pdkim.c
+++ b/src/src/pdkim/pdkim.c
@@ -1056,13 +1056,13 @@ else for (p = 0; p<len; p++)
 	if ((rc = pdkim_header_complete(ctx)) != PDKIM_OK)
 	  return rc;
 
-	ctx->flags = ctx->flags & ~(PDKIM_SEEN_LF|PDKIM_SEEN_CR) | PDKIM_PAST_HDRS;
+	ctx->flags = (ctx->flags & ~(PDKIM_SEEN_LF|PDKIM_SEEN_CR)) | PDKIM_PAST_HDRS;
 	DEBUG(D_acl) debug_printf(
 	    "PDKIM >> Body data for hash, canonicalized >>>>>>>>>>>>>>>>>>>>>>>>>>>>\n");
 	continue;
 	}
       else
-	ctx->flags = ctx->flags & ~PDKIM_SEEN_CR | PDKIM_SEEN_LF;
+	ctx->flags = (ctx->flags & ~PDKIM_SEEN_CR) | PDKIM_SEEN_LF;
       }
     else if (ctx->flags & PDKIM_SEEN_LF)
       {
diff --git a/src/src/readconf.c b/src/src/readconf.c
index 7751b3607..5efe7aa04 100644
--- a/src/src/readconf.c
+++ b/src/src/readconf.c
@@ -877,7 +877,7 @@ of the macros list is in reverse-alpha (we prepend them) - so longer
 macros that have substrings are always discovered first during
 expansion. */
 
-for (i = 0; i < nopt; i++)  if (*(s = opts[i].name) && *s != '*')
+for (i = 0; i < nopt; i++)  if (*(s = US opts[i].name) && *s != '*')
   if (group)
     macro_create(string_sprintf("_OPT_%T_%T_%T", section, group, s), US"y", FALSE, TRUE);
   else
@@ -1201,7 +1201,7 @@ for (;;)
         "configuration file %s", ss);
 
     config_filename = string_copy(ss);
-    config_directory = string_copyn(ss, (const uschar*) strrchr(ss, '/') - ss);
+    config_directory = string_copyn(ss, CUstrrchr(ss, '/') - ss);
     config_lineno = 0;
     continue;
     }
@@ -3391,11 +3391,11 @@ if (config_file)
     {
       /* relative configuration file name: working dir + / + basename(filename) */
 
-      char buf[PATH_MAX];
+      uschar buf[PATH_MAX];
       int offset = 0;
       int size = 0;
 
-      if (getcwd(buf, PATH_MAX) == NULL)
+      if (os_getcwd(buf, PATH_MAX) == NULL)
         {
         perror("exim: getcwd");
         exit(EXIT_FAILURE);
diff --git a/src/src/transports/smtp.c b/src/src/transports/smtp.c
index a4c036642..cbd09c00c 100644
--- a/src/src/transports/smtp.c
+++ b/src/src/transports/smtp.c
@@ -1551,7 +1551,7 @@ if (continue_hostname == NULL)
     uschar * msg = NULL;
     if (sx->verify)
       {
-      msg = strerror(errno);
+      msg = US strerror(errno);
       HDEBUG(D_verify) debug_printf("connect: %s\n", msg);
       }
     set_errno_nohost(sx->addrlist,