From 9aef4157fdd83cc16674dd359fb4aee31ea6a925 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Wed, 26 Mar 2014 12:27:35 +0000 Subject: [PATCH] Fix review comments from dl . Add __attribute__((__format__(printf))) to functions that take a printf-compatible format string (when building using GCC or Clang), so that format string mistakes can be detected statically. Copied from Perforce Change: 185021 ServerID: perforce.ravenbrook.com --- mps/code/awlut.c | 6 ++++-- mps/code/awluthe.c | 8 +++++--- mps/code/config.h | 17 +++++++++++++++++ mps/code/eventcnv.c | 3 +++ mps/code/eventrep.c | 1 + mps/code/eventsql.c | 13 ++++++++----- mps/code/eventtxt.c | 1 + mps/code/mpswin.h | 1 + mps/code/poolmv2.c | 4 +++- mps/code/replay.c | 1 + mps/code/teletest.c | 2 +- mps/code/testlib.c | 2 ++ mps/code/testlib.h | 19 +++++++++++++++++++ mps/code/w3i3pc.nmk | 3 --- mps/code/w3i6pc.nmk | 3 --- 15 files changed, 66 insertions(+), 18 deletions(-) diff --git a/mps/code/awlut.c b/mps/code/awlut.c index e5dbd64a47e..927d16add46 100644 --- a/mps/code/awlut.c +++ b/mps/code/awlut.c @@ -221,11 +221,13 @@ static void test(mps_arena_t arena, for(i = 0; i < TABLE_SLOTS; ++i) { if (preserve[i] == 0) { if (table_slot(weaktable, i)) { - error("Strongly unreachable weak table entry found, slot %lu.\n", i); + error("Strongly unreachable weak table entry found, " + "slot %"PRIuLONGEST".\n", (ulongest_t)i); } else { if (table_slot(exacttable, i) != 0) { error("Weak table entry deleted, but corresponding " - "exact table entry not deleted, slot %lu.\n", i); + "exact table entry not deleted, slot %"PRIuLONGEST".\n", + (ulongest_t)i); } } } diff --git a/mps/code/awluthe.c b/mps/code/awluthe.c index fa883a30487..257b4142723 100644 --- a/mps/code/awluthe.c +++ b/mps/code/awluthe.c @@ -121,7 +121,7 @@ static mps_word_t *alloc_string(const char *s, mps_ap_t ap) * .assume.dylan-obj */ -static mps_word_t *alloc_table(unsigned long n, mps_ap_t ap) +static mps_word_t *alloc_table(size_t n, mps_ap_t ap) { size_t objsize; void *p; @@ -226,11 +226,13 @@ static void test(mps_arena_t arena, for(i = 0; i < TABLE_SLOTS; ++i) { if (preserve[i] == 0) { if (table_slot(weaktable, i)) { - error("Strongly unreachable weak table entry found, slot %lu.\n", i); + error("Strongly unreachable weak table entry found, " + "slot %"PRIuLONGEST".\n", (ulongest_t)i); } else { if (table_slot(exacttable, i) != 0) { error("Weak table entry deleted, but corresponding " - "exact table entry not deleted, slot %lu.\n", i); + "exact table entry not deleted, slot %"PRIuLONGEST".\n", + (ulongest_t)i); } } } diff --git a/mps/code/config.h b/mps/code/config.h index d66487e1684..cfca6ab01b6 100644 --- a/mps/code/config.h +++ b/mps/code/config.h @@ -225,6 +225,23 @@ #endif /* MPS_BUILD_PC */ +/* Function attributes */ +/* These are also defined in testlib.h */ + +#if defined(MPS_BUILD_GC) || defined(MPS_BUILD_LL) + +/* GCC: + * Clang: + */ +#define ATTRIBUTE_FORMAT(ARGLIST) __attribute__((__format__ ARGLIST)) + +#else + +#define ATTRIBUTE_FORMAT(ARGLIST) + +#endif + + /* EPVMDefaultSubsequentSegSIZE is a default for the alignment of * subsequent segments (non-initial at each save level) in EPVM. See * design.mps.poolepvm.arch.segment.size. diff --git a/mps/code/eventcnv.c b/mps/code/eventcnv.c index 33375c45f4d..3464605cf1e 100644 --- a/mps/code/eventcnv.c +++ b/mps/code/eventcnv.c @@ -62,6 +62,7 @@ static const char *prog; /* program name */ /* fevwarn -- flush stdout, write message to stderr */ +ATTRIBUTE_FORMAT((printf, 2, 0)) static void fevwarn(const char *prefix, const char *format, va_list args) { (void)fflush(stdout); /* sync */ @@ -74,6 +75,7 @@ static void fevwarn(const char *prefix, const char *format, va_list args) /* evwarn -- flush stdout, warn to stderr */ +ATTRIBUTE_FORMAT((printf, 1, 2)) static void evwarn(const char *format, ...) { va_list args; @@ -85,6 +87,7 @@ static void evwarn(const char *format, ...) /* everror -- flush stdout, message to stderr, exit */ +ATTRIBUTE_FORMAT((printf, 1, 2)) static void everror(const char *format, ...) { va_list args; diff --git a/mps/code/eventrep.c b/mps/code/eventrep.c index c2364529cf1..375d793c13f 100644 --- a/mps/code/eventrep.c +++ b/mps/code/eventrep.c @@ -116,6 +116,7 @@ typedef struct apRepStruct *apRep; /* error -- error signalling */ +ATTRIBUTE_FORMAT((printf, 1, 2)) static void error(const char *format, ...) { va_list args; diff --git a/mps/code/eventsql.c b/mps/code/eventsql.c index 0fa36dea739..74aaad92b31 100644 --- a/mps/code/eventsql.c +++ b/mps/code/eventsql.c @@ -113,6 +113,7 @@ unsigned int verbosity = 0; #define LOG_SELDOM 3 #define LOG_RARELY 4 +ATTRIBUTE_FORMAT((printf, 2, 0)) static void vlog(unsigned int level, const char *format, va_list args) { if (level <= verbosity) { @@ -123,6 +124,7 @@ static void vlog(unsigned int level, const char *format, va_list args) } } +ATTRIBUTE_FORMAT((printf, 2, 3)) static void evlog(unsigned int level, const char *format, ...) { va_list args; @@ -131,6 +133,7 @@ static void evlog(unsigned int level, const char *format, ...) va_end(args); } +ATTRIBUTE_FORMAT((printf, 1, 2)) static void error(const char *format, ...) { va_list args; @@ -452,7 +455,7 @@ static void registerLogFile(sqlite3 *db, name = sqlite3_column_text(statement, 0); logSerial = sqlite3_column_int64(statement, 1); completed = sqlite3_column_int64(statement, 2); - evlog(force ? LOG_OFTEN : LOG_ALWAYS, "Log file matching '%s' already in event_log, named \"%s\" (serial %lu, completed %lu).", + evlog(force ? LOG_OFTEN : LOG_ALWAYS, "Log file matching '%s' already in event_log, named \"%s\" (serial %llu, completed %llu).", filename, name, logSerial, completed); if (force) { evlog(LOG_OFTEN, "Continuing anyway because -f specified."); @@ -486,7 +489,7 @@ static void registerLogFile(sqlite3 *db, if (res != SQLITE_DONE) sqlite_error(res, db, "insert into event_log failed."); logSerial = sqlite3_last_insert_rowid(db); - evlog(LOG_SOMETIMES, "Log file %s added to event_log with serial %lu", + evlog(LOG_SOMETIMES, "Log file %s added to event_log with serial %llu", filename, logSerial); finalizeStatement(db, statement); } @@ -508,7 +511,7 @@ static void logFileCompleted(sqlite3 *db, res = sqlite3_step(statement); if (res != SQLITE_DONE) sqlite_error(res, db, "insert into event_log failed."); - evlog(LOG_SOMETIMES, "Marked in event_log: %lu events", completed); + evlog(LOG_SOMETIMES, "Marked in event_log: %llu events", completed); finalizeStatement(db, statement); } @@ -864,7 +867,7 @@ static int64 readLog(FILE *input, /* this macro sets statement and last_index */ EVENT_LIST(EVENT_TYPE_WRITE_SQL, X); default: - error("Event %llu has Unknown event code %d", eventCount, code); + error("Event %llu has Unknown event code %ld", eventCount, code); } /* bind the fields we store for every event */ \ res = sqlite3_bind_int64(statement, last_index+1, logSerial); @@ -951,7 +954,7 @@ int main(int argc, char *argv[]) makeTables(db); fillGlueTables(db); count = writeEventsToSQL(db); - evlog(LOG_ALWAYS, "Imported %llu events from %s to %s, serial %lu.", + evlog(LOG_ALWAYS, "Imported %llu events from %s to %s, serial %llu.", count, logFileName, databaseName, logSerial); if (runTests) { diff --git a/mps/code/eventtxt.c b/mps/code/eventtxt.c index 4782e576a94..4e81a8b06c4 100644 --- a/mps/code/eventtxt.c +++ b/mps/code/eventtxt.c @@ -50,6 +50,7 @@ static const char *logFileName = NULL; /* everror -- error signalling */ +ATTRIBUTE_FORMAT((printf, 1, 2)) static void everror(const char *format, ...) { va_list args; diff --git a/mps/code/mpswin.h b/mps/code/mpswin.h index 1e25267c4ee..8d54f5c90a4 100644 --- a/mps/code/mpswin.h +++ b/mps/code/mpswin.h @@ -20,6 +20,7 @@ * don't use. See */ #define WIN32_LEAN_AND_MEAN #include +#undef WIN32_LEAN_AND_MEAN #ifdef MPS_BUILD_MV #pragma warning(default: 4115 4201 4209 4214) diff --git a/mps/code/poolmv2.c b/mps/code/poolmv2.c index d244fe23043..d9d063dc39e 100644 --- a/mps/code/poolmv2.c +++ b/mps/code/poolmv2.c @@ -280,7 +280,7 @@ static Res MVTInit(Pool pool, ArgList args) res = FreelistInit(MVTFreelist(mvt), align); if (res != ResOK) - goto failABQ; + goto failFreelist; pool->alignment = align; mvt->reuseSize = reuseSize; @@ -345,6 +345,8 @@ static Res MVTInit(Pool pool, ArgList args) reserveDepth, fragLimit); return ResOK; +failFreelist: + ABQFinish(arena, MVTABQ(mvt)); failABQ: CBSFinish(MVTCBS(mvt)); failCBS: diff --git a/mps/code/replay.c b/mps/code/replay.c index 0069e7415c1..9f761119c4f 100644 --- a/mps/code/replay.c +++ b/mps/code/replay.c @@ -47,6 +47,7 @@ static Word eventTime = 0; /* current event time */ /* error -- error signalling */ +ATTRIBUTE_FORMAT((printf, 1, 2)) static void error(const char *format, ...) { va_list args; diff --git a/mps/code/teletest.c b/mps/code/teletest.c index 312237f5251..07d2948513c 100644 --- a/mps/code/teletest.c +++ b/mps/code/teletest.c @@ -19,7 +19,7 @@ SRCID(teletest, "$Id$"); static mps_arena_t arena; -#define WORD_FORMAT "%" PRIwWORD PRIuLONGEST +#define WORD_FORMAT "0x%0" PRIwWORD PRIuLONGEST #define MAX_ARGS 3 #define INPUT_BUFFER_SIZE 512 diff --git a/mps/code/testlib.c b/mps/code/testlib.c index d4cf0afc0f2..6f5a6dd2ff6 100644 --- a/mps/code/testlib.c +++ b/mps/code/testlib.c @@ -339,6 +339,7 @@ _mps_RES_ENUM(RES_STRINGS_ROW, X) /* verror -- die with message */ +ATTRIBUTE_FORMAT((printf, 1, 0)) void verror(const char *format, va_list args) { (void)fflush(stdout); /* synchronize */ @@ -360,6 +361,7 @@ void verror(const char *format, va_list args) /* error -- die with message */ +ATTRIBUTE_FORMAT((printf, 1, 2)) void error(const char *format, ...) { va_list args; diff --git a/mps/code/testlib.h b/mps/code/testlib.h index 6dea9beae75..252fb5167a3 100644 --- a/mps/code/testlib.h +++ b/mps/code/testlib.h @@ -83,6 +83,23 @@ #endif /* MPS_BUILD_PC */ +/* Function attributes */ +/* These are also defined in config.h */ + +#if defined(MPS_BUILD_GC) || defined(MPS_BUILD_LL) + +/* GCC: + * Clang: + */ +#define ATTRIBUTE_FORMAT(ARGLIST) __attribute__((__format__ ARGLIST)) + +#else + +#define ATTRIBUTE_FORMAT(ARGLIST) + +#endif + + /* ulongest_t -- longest unsigned integer type * * Define a longest unsigned integer type for testing, scanning, and @@ -195,7 +212,9 @@ void assert_die(const char *file, unsigned line, const char *condition); /* error, verror -- die with message */ +ATTRIBUTE_FORMAT((printf, 1, 2)) extern void error(const char *format, ...); +ATTRIBUTE_FORMAT((printf, 1, 0)) extern void verror(const char *format, va_list args); diff --git a/mps/code/w3i3pc.nmk b/mps/code/w3i3pc.nmk index ce7358df2ab..d6d424b669d 100644 --- a/mps/code/w3i3pc.nmk +++ b/mps/code/w3i3pc.nmk @@ -5,9 +5,6 @@ PFM = w3i3pc -# /Gs appears to be necessary to suppress stack checks. Stack checks -# (if not suppressed) generate a dependency on the C library, __chkesp, -# which causes the linker step to fail when building the DLL, mpsdy.dll. PFMDEFS = /DCONFIG_PF_STRING="w3i3pc" /DCONFIG_PF_W3I3PC /DWIN32 /D_WINDOWS !INCLUDE commpre.nmk diff --git a/mps/code/w3i6pc.nmk b/mps/code/w3i6pc.nmk index fd3f798a9b5..1decdde0083 100644 --- a/mps/code/w3i6pc.nmk +++ b/mps/code/w3i6pc.nmk @@ -7,9 +7,6 @@ PFM = w3i6pc -# /Gs appears to be necessary to suppress stack checks. Stack checks -# (if not suppressed) generate a dependency on the C library, __chkesp, -# which causes the linker step to fail when building the DLL, mpsdy.dll. PFMDEFS = /DCONFIG_PF_STRING="w3i6pc" /DCONFIG_PF_W3I6PC /DWIN32 /D_WINDOWS !INCLUDE commpre.nmk