1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-09 17:38:00 +00:00

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.
This commit is contained in:
Simon Tatham 2020-01-26 14:49:31 +00:00
parent 0a4e068ada
commit cbfba7a0e9
15 changed files with 62 additions and 56 deletions

View File

@ -1205,7 +1205,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
@ -1303,7 +1303,7 @@ char *get_fp(char *filename, unsigned flags)
return toret;
}
void check_fp(char *filename, char *fp, char *fmt, ...)
PRINTF_LIKE(3, 4) void check_fp(char *filename, char *fp, char *fmt, ...)
{
char *newfp;

20
defs.h
View File

@ -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;

View File

@ -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;

26
misc.h
View File

@ -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);
@ -267,7 +247,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))

View File

@ -268,7 +268,8 @@ char *get_hostname(void);
* The consume_string variant takes an already-formatted dynamically
* allocated string, and takes over ownership of that string.
*/
Socket *new_error_socket_fmt(Plug *plug, const char *fmt, ...);
Socket *new_error_socket_fmt(Plug *plug, const char *fmt, ...)
PRINTF_LIKE(2, 3);
Socket *new_error_socket_consume_string(Plug *plug, char *errmsg);
/*

View File

@ -308,7 +308,8 @@ void pageant_unregister_client(PageantClient *pc)
sfree(pc->info);
}
static void failure(PageantClient *pc, PageantClientRequestId *reqid,
static PRINTF_LIKE(4, 5) void failure(PageantClient *pc,
PageantClientRequestId *reqid,
strbuf *sb, const char *fmt, ...)
{
strbuf_clear(sb);

View File

@ -41,7 +41,7 @@ static inline void pageant_client_log_v(
if (!pc->suppress_logging)
pc->vt->log(pc, reqid, fmt, ap);
}
static inline void pageant_client_log(
static inline PRINTF_LIKE(3, 4) void pageant_client_log(
PageantClient *pc, PageantClientRequestId *reqid, const char *fmt, ...)
{
if (!pc->suppress_logging) {
@ -153,7 +153,7 @@ static inline void pageant_listener_client_log_v(
if (!plc->suppress_logging)
plc->vt->log(plc, fmt, ap);
}
static inline void pageant_listener_client_log(
static inline PRINTF_LIKE(2, 3) void pageant_listener_client_log(
PageantListenerClient *plc, const char *fmt, ...)
{
if (!plc->suppress_logging) {

6
pscp.c
View File

@ -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;

13
putty.h
View File

@ -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);
/*
@ -1927,7 +1927,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.
@ -1965,7 +1966,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.

View File

@ -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);

12
ssh.h
View File

@ -406,12 +406,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

View File

@ -705,7 +705,7 @@ static void share_remove_forwarding(struct ssh_sharing_connstate *cs,
sfree(fwd);
}
static void log_downstream(struct ssh_sharing_connstate *cs,
static PRINTF_LIKE(2, 3) void log_downstream(struct ssh_sharing_connstate *cs,
const char *logfmt, ...)
{
va_list ap;
@ -719,7 +719,7 @@ static void log_downstream(struct ssh_sharing_connstate *cs,
sfree(buf);
}
static void log_general(struct ssh_sharing_state *sharestate,
static PRINTF_LIKE(2, 3) void log_general(struct ssh_sharing_state *sharestate,
const char *logfmt, ...)
{
va_list ap;

View File

@ -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: ");

View File

@ -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: ");

View File

@ -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: ");