From 03f6e88385d44150245936fdb6b4d07279183470 Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Sun, 9 Feb 2020 08:22:20 +0000
Subject: [PATCH] Greatly improve printf format-string checking.

I've added the gcc-style attribute("printf") to a lot of printf-shaped
functions in this code base that didn't have it. To make that easier,
I moved the wrapping macro into defs.h, and also enabled it if we
detect the __clang__ macro as well as __GNU__ (hence, it will be used
when building for Windows using clang-cl).

The result is that a great many format strings in the code are now
checked by the compiler, where they were previously not. This causes
build failures, which I'll fix in the next commit.

(cherry picked from commit cbfba7a0e954fe0f2c3602594fa609be21a35833)
---
 cmdgen.c    |  4 ++--
 defs.h      | 20 ++++++++++++++++++++
 logging.c   |  2 +-
 misc.h      | 26 +++-----------------------
 network.h   |  3 ++-
 pageant.c   |  9 ++-------
 pscp.c      |  6 +++---
 putty.h     | 13 +++++++------
 scpserver.c |  9 ++++++---
 ssh.h       | 12 ++++++------
 sshshare.c  |  8 ++++----
 testcrypt.c |  2 +-
 testsc.c    |  2 +-
 tree234.c   |  2 +-
 14 files changed, 59 insertions(+), 59 deletions(-)

diff --git a/cmdgen.c b/cmdgen.c
index 03a1eab2..b391bd1d 100644
--- a/cmdgen.c
+++ b/cmdgen.c
@@ -1195,7 +1195,7 @@ void test(int retval, ...)
     sfree(argv);
 }
 
-void filecmp(char *file1, char *file2, char *fmt, ...)
+PRINTF_LIKE(3, 4) void filecmp(char *file1, char *file2, char *fmt, ...)
 {
     /*
      * Ideally I should do file comparison myself, to maximise the
@@ -1260,7 +1260,7 @@ char *get_fp(char *filename)
     return cleanup_fp(buf);
 }
 
-void check_fp(char *filename, char *fp, char *fmt, ...)
+PRINTF_LIKE(3, 4) void check_fp(char *filename, char *fp, char *fmt, ...)
 {
     char *newfp;
 
diff --git a/defs.h b/defs.h
index afeb5062..0a4f08c8 100644
--- a/defs.h
+++ b/defs.h
@@ -27,6 +27,26 @@ uintmax_t strtoumax(const char *nptr, char **endptr, int base);
 #include <inttypes.h>
 #endif
 
+#if defined __GNUC__ || defined __clang__
+/*
+ * On MinGW, the correct compiler format checking for vsnprintf() etc
+ * can depend on compile-time flags; these control whether you get
+ * ISO C or Microsoft's non-standard format strings.
+ * We sometimes use __attribute__ ((format)) for our own printf-like
+ * functions, which are ultimately interpreted by the toolchain-chosen
+ * printf, so we need to take that into account to get correct warnings.
+ */
+#ifdef __MINGW_PRINTF_FORMAT
+#define PRINTF_LIKE(fmt_index, ellipsis_index) \
+    __attribute__ ((format (__MINGW_PRINTF_FORMAT, fmt_index, ellipsis_index)))
+#else
+#define PRINTF_LIKE(fmt_index, ellipsis_index) \
+    __attribute__ ((format (printf, fmt_index, ellipsis_index)))
+#endif
+#else /* __GNUC__ */
+#define PRINTF_LIKE(fmt_index, ellipsis_index)
+#endif /* __GNUC__ */
+
 typedef struct conf_tag Conf;
 typedef struct terminal_tag Terminal;
 typedef struct term_utf8_decode term_utf8_decode;
diff --git a/logging.c b/logging.c
index 173ed080..e324c77c 100644
--- a/logging.c
+++ b/logging.c
@@ -58,7 +58,7 @@ static void logwrite(LogContext *ctx, ptrlen data)
  * Convenience wrapper on logwrite() which printf-formats the
  * string.
  */
-static void logprintf(LogContext *ctx, const char *fmt, ...)
+static PRINTF_LIKE(2, 3) void logprintf(LogContext *ctx, const char *fmt, ...)
 {
     va_list ap;
     char *data;
diff --git a/misc.h b/misc.h
index 590c8a3b..d75243af 100644
--- a/misc.h
+++ b/misc.h
@@ -24,30 +24,10 @@ char *host_strchr(const char *s, int c);
 char *host_strrchr(const char *s, int c);
 char *host_strduptrim(const char *s);
 
-#ifdef __GNUC__
-/*
- * On MinGW, the correct compiler format checking for vsnprintf() etc
- * can depend on compile-time flags; these control whether you get
- * ISO C or Microsoft's non-standard format strings.
- * We sometimes use __attribute__ ((format)) for our own printf-like
- * functions, which are ultimately interpreted by the toolchain-chosen
- * printf, so we need to take that into account to get correct warnings.
- */
-#ifdef __MINGW_PRINTF_FORMAT
-#define PUTTY_PRINTF_ARCHETYPE __MINGW_PRINTF_FORMAT
-#else
-#define PUTTY_PRINTF_ARCHETYPE printf
-#endif
-#endif /* __GNUC__ */
-
 char *dupstr(const char *s);
 char *dupcat_fn(const char *s1, ...);
 #define dupcat(...) dupcat_fn(__VA_ARGS__, (const char *)NULL)
-char *dupprintf(const char *fmt, ...)
-#ifdef __GNUC__
-    __attribute__ ((format (PUTTY_PRINTF_ARCHETYPE, 1, 2)))
-#endif
-    ;
+char *dupprintf(const char *fmt, ...) PRINTF_LIKE(1, 2);
 char *dupvprintf(const char *fmt, va_list ap);
 void burnstr(char *string);
 
@@ -76,7 +56,7 @@ void *strbuf_append(strbuf *buf, size_t len);
 void strbuf_shrink_to(strbuf *buf, size_t new_len);
 void strbuf_shrink_by(strbuf *buf, size_t amount_to_remove);
 char *strbuf_to_str(strbuf *buf); /* does free buf, but you must free result */
-void strbuf_catf(strbuf *buf, const char *fmt, ...);
+void strbuf_catf(strbuf *buf, const char *fmt, ...) PRINTF_LIKE(2, 3);
 void strbuf_catfv(strbuf *buf, const char *fmt, va_list ap);
 static inline void strbuf_clear(strbuf *buf) { strbuf_shrink_to(buf, 0); }
 bool strbuf_chomp(strbuf *buf, char char_to_remove);
@@ -263,7 +243,7 @@ static inline NORETURN void unreachable_internal(void) { abort(); }
  */
 
 #ifdef DEBUG
-void debug_printf(const char *fmt, ...);
+void debug_printf(const char *fmt, ...) PRINTF_LIKE(1, 2);
 void debug_memdump(const void *buf, int len, bool L);
 #define debug(...) (debug_printf(__VA_ARGS__))
 #define dmemdump(buf,len) (debug_memdump(buf, len, false))
diff --git a/network.h b/network.h
index 355da2b3..49d517d2 100644
--- a/network.h
+++ b/network.h
@@ -265,7 +265,8 @@ char *get_hostname(void);
  * Trivial socket implementation which just stores an error. Found in
  * errsock.c.
  */
-Socket *new_error_socket_fmt(Plug *plug, const char *fmt, ...);
+Socket *new_error_socket_fmt(Plug *plug, const char *fmt, ...)
+    PRINTF_LIKE(2, 3);
 
 /*
  * Trivial plug that does absolutely nothing. Found in nullplug.c.
diff --git a/pageant.c b/pageant.c
index 70b21743..ba334cc3 100644
--- a/pageant.c
+++ b/pageant.c
@@ -129,13 +129,8 @@ void pageant_make_keylist2(BinarySink *bs)
     }
 }
 
-static void plog(void *logctx, pageant_logfn_t logfn, const char *fmt, ...)
-#ifdef __GNUC__
-__attribute__ ((format (PUTTY_PRINTF_ARCHETYPE, 3, 4)))
-#endif
-    ;
-
-static void plog(void *logctx, pageant_logfn_t logfn, const char *fmt, ...)
+static PRINTF_LIKE(3, 4) void plog(void *logctx, pageant_logfn_t logfn,
+                                   const char *fmt, ...)
 {
     /*
      * This is the wrapper that takes a variadic argument list and
diff --git a/pscp.c b/pscp.c
index ef65a591..5f69e4c3 100644
--- a/pscp.c
+++ b/pscp.c
@@ -113,7 +113,7 @@ static void abandon_stats(void)
     }
 }
 
-static void tell_user(FILE *stream, const char *fmt, ...)
+static PRINTF_LIKE(2, 3) void tell_user(FILE *stream, const char *fmt, ...)
 {
     char *str, *str2;
     va_list ap;
@@ -224,7 +224,7 @@ static void ssh_scp_init(void)
 /*
  *  Print an error message and exit after closing the SSH link.
  */
-static NORETURN void bump(const char *fmt, ...)
+static NORETURN PRINTF_LIKE(1, 2) void bump(const char *fmt, ...)
 {
     char *str, *str2;
     va_list ap;
@@ -1541,7 +1541,7 @@ int scp_finish_filerecv(void)
  *  Send an error message to the other side and to the screen.
  *  Increment error counter.
  */
-static void run_err(const char *fmt, ...)
+static PRINTF_LIKE(1, 2) void run_err(const char *fmt, ...)
 {
     char *str, *str2;
     va_list ap;
diff --git a/putty.h b/putty.h
index 5fb90366..1498370e 100644
--- a/putty.h
+++ b/putty.h
@@ -1003,7 +1003,7 @@ static inline bool seat_set_trust_status(Seat *seat, bool trusted)
 /* Unlike the seat's actual method, the public entry point
  * seat_connection_fatal is a wrapper function with a printf-like API,
  * defined in misc.c. */
-void seat_connection_fatal(Seat *seat, const char *fmt, ...);
+void seat_connection_fatal(Seat *seat, const char *fmt, ...) PRINTF_LIKE(2, 3);
 
 /* Handy aliases for seat_output which set is_stderr to a fixed value. */
 static inline size_t seat_stdout(Seat *seat, const void *data, size_t len)
@@ -1218,8 +1218,8 @@ static inline bool win_is_utf8(TermWin *win)
 /*
  * Global functions not specific to a connection instance.
  */
-void nonfatal(const char *, ...);
-NORETURN void modalfatalbox(const char *, ...);
+void nonfatal(const char *, ...) PRINTF_LIKE(1, 2);
+NORETURN void modalfatalbox(const char *, ...) PRINTF_LIKE(1, 2);
 NORETURN void cleanup_exit(int);
 
 /*
@@ -1703,7 +1703,7 @@ void logfclose(LogContext *logctx);
 void logtraffic(LogContext *logctx, unsigned char c, int logmode);
 void logflush(LogContext *logctx);
 void logevent(LogContext *logctx, const char *event);
-void logeventf(LogContext *logctx, const char *fmt, ...);
+void logeventf(LogContext *logctx, const char *fmt, ...) PRINTF_LIKE(2, 3);
 void logeventvf(LogContext *logctx, const char *fmt, va_list ap);
 
 /*
@@ -1917,7 +1917,8 @@ bool is_interactive(void);
 void console_print_error_msg(const char *prefix, const char *msg);
 void console_print_error_msg_fmt_v(
     const char *prefix, const char *fmt, va_list ap);
-void console_print_error_msg_fmt(const char *prefix, const char *fmt, ...);
+void console_print_error_msg_fmt(const char *prefix, const char *fmt, ...)
+    PRINTF_LIKE(2, 3);
 
 /*
  * Exports from printing.c.
@@ -1955,7 +1956,7 @@ bool cmdline_host_ok(Conf *);
 #define TOOLTYPE_PORT_ARG 64
 extern int cmdline_tooltype;
 
-void cmdline_error(const char *, ...);
+void cmdline_error(const char *, ...) PRINTF_LIKE(1, 2);
 
 /*
  * Exports from config.c.
diff --git a/scpserver.c b/scpserver.c
index e594f1ab..3a136286 100644
--- a/scpserver.c
+++ b/scpserver.c
@@ -431,7 +431,8 @@ static char *scp_source_err_base(ScpSource *scp, const char *fmt, va_list ap)
     sshfwd_write_ext(scp->sc, true, "\012", 1);
     return msg;
 }
-static void scp_source_err(ScpSource *scp, const char *fmt, ...)
+static PRINTF_LIKE(2, 3) void scp_source_err(
+    ScpSource *scp, const char *fmt, ...)
 {
     va_list ap;
 
@@ -439,7 +440,8 @@ static void scp_source_err(ScpSource *scp, const char *fmt, ...)
     sfree(scp_source_err_base(scp, fmt, ap));
     va_end(ap);
 }
-static void scp_source_abort(ScpSource *scp, const char *fmt, ...)
+static PRINTF_LIKE(2, 3) void scp_source_abort(
+    ScpSource *scp, const char *fmt, ...)
 {
     va_list ap;
     char *msg;
@@ -1312,7 +1314,8 @@ static void scp_error_send_message_cb(void *vscp)
     sshfwd_initiate_close(scp->sc, scp->message);
 }
 
-static ScpError *scp_error_new(SshChannel *sc, const char *fmt, ...)
+static PRINTF_LIKE(2, 3) ScpError *scp_error_new(
+    SshChannel *sc, const char *fmt, ...)
 {
     va_list ap;
     ScpError *scp = snew(ScpError);
diff --git a/ssh.h b/ssh.h
index af0f9d47..d3c31c8f 100644
--- a/ssh.h
+++ b/ssh.h
@@ -403,12 +403,12 @@ void ssh_conn_processed_data(Ssh *ssh);
 void ssh_check_frozen(Ssh *ssh);
 
 /* Functions to abort the connection, for various reasons. */
-void ssh_remote_error(Ssh *ssh, const char *fmt, ...);
-void ssh_remote_eof(Ssh *ssh, const char *fmt, ...);
-void ssh_proto_error(Ssh *ssh, const char *fmt, ...);
-void ssh_sw_abort(Ssh *ssh, const char *fmt, ...);
-void ssh_sw_abort_deferred(Ssh *ssh, const char *fmt, ...);
-void ssh_user_close(Ssh *ssh, const char *fmt, ...);
+void ssh_remote_error(Ssh *ssh, const char *fmt, ...) PRINTF_LIKE(2, 3);
+void ssh_remote_eof(Ssh *ssh, const char *fmt, ...) PRINTF_LIKE(2, 3);
+void ssh_proto_error(Ssh *ssh, const char *fmt, ...) PRINTF_LIKE(2, 3);
+void ssh_sw_abort(Ssh *ssh, const char *fmt, ...) PRINTF_LIKE(2, 3);
+void ssh_sw_abort_deferred(Ssh *ssh, const char *fmt, ...) PRINTF_LIKE(2, 3);
+void ssh_user_close(Ssh *ssh, const char *fmt, ...) PRINTF_LIKE(2, 3);
 
 /* Bit positions in the SSH-1 cipher protocol word */
 #define SSH1_CIPHER_IDEA        1
diff --git a/sshshare.c b/sshshare.c
index 24f4c125..98071d8e 100644
--- a/sshshare.c
+++ b/sshshare.c
@@ -705,8 +705,8 @@ static void share_remove_forwarding(struct ssh_sharing_connstate *cs,
     sfree(fwd);
 }
 
-static void log_downstream(struct ssh_sharing_connstate *cs,
-                           const char *logfmt, ...)
+static PRINTF_LIKE(2, 3) void log_downstream(struct ssh_sharing_connstate *cs,
+                                             const char *logfmt, ...)
 {
     va_list ap;
     char *buf;
@@ -719,8 +719,8 @@ static void log_downstream(struct ssh_sharing_connstate *cs,
     sfree(buf);
 }
 
-static void log_general(struct ssh_sharing_state *sharestate,
-                        const char *logfmt, ...)
+static PRINTF_LIKE(2, 3) void log_general(struct ssh_sharing_state *sharestate,
+                                          const char *logfmt, ...)
 {
     va_list ap;
     char *buf;
diff --git a/testcrypt.c b/testcrypt.c
index ec7356f6..c92311f4 100644
--- a/testcrypt.c
+++ b/testcrypt.c
@@ -35,7 +35,7 @@
 #include "mpint.h"
 #include "ecc.h"
 
-static NORETURN void fatal_error(const char *p, ...)
+static NORETURN PRINTF_LIKE(1, 2) void fatal_error(const char *p, ...)
 {
     va_list ap;
     fprintf(stderr, "testcrypt: ");
diff --git a/testsc.c b/testsc.c
index 79c5ba1f..760a9482 100644
--- a/testsc.c
+++ b/testsc.c
@@ -81,7 +81,7 @@
 #include "mpint.h"
 #include "ecc.h"
 
-static NORETURN void fatal_error(const char *p, ...)
+static NORETURN PRINTF_LIKE(1, 2) void fatal_error(const char *p, ...)
 {
     va_list ap;
     fprintf(stderr, "testsc: ");
diff --git a/tree234.c b/tree234.c
index 0ec2b520..bf68e23f 100644
--- a/tree234.c
+++ b/tree234.c
@@ -1072,7 +1072,7 @@ int n_errors = 0;
 /*
  * Error reporting function.
  */
-void error(char *fmt, ...)
+PRINTF_LIKE(1, 2) void error(char *fmt, ...)
 {
     va_list ap;
     printf("ERROR: ");