From 928963656d68dbface669773eecfb6b22ba15da5 Mon Sep 17 00:00:00 2001 From: Simon Howard Date: Sat, 1 May 2010 19:22:52 +0000 Subject: Fix compiler warnings with savegame and response file code. Subversion-branch: /trunk/chocolate-doom Subversion-revision: 1926 --- src/g_game.c | 4 ++++ src/m_argv.c | 9 +++++++-- src/p_saveg.c | 22 ++++++++++++++++++++-- src/p_saveg.h | 1 + 4 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/g_game.c b/src/g_game.c index 542c1a7d..b0532f42 100644 --- a/src/g_game.c +++ b/src/g_game.c @@ -1490,6 +1490,8 @@ void G_DoLoadGame (void) return; } + savegame_error = false; + if (!P_ReadSaveGameHeader()) { fclose(save_stream); @@ -1557,6 +1559,8 @@ void G_DoSaveGame (void) return; } + savegame_error = false; + P_WriteSaveGameHeader(savedescription); P_ArchivePlayers (); diff --git a/src/m_argv.c b/src/m_argv.c index 78404d35..7fc15863 100644 --- a/src/m_argv.c +++ b/src/m_argv.c @@ -89,12 +89,17 @@ static void LoadResponseFile(int argv_index) size = M_FileLength(handle); // Read in the entire file - // Allocate one byte extra - this is incase there is an argument + // Allocate one byte extra - this is in case there is an argument // at the end of the response file, in which case a '\0' will be // needed. file = malloc(size + 1); - fread(file, size, 1, handle); + + if (fread(file, 1, size, handle) < size) + { + I_Error("Failed to read entire response file"); + } + fclose(handle); // Create new arguments list array diff --git a/src/p_saveg.c b/src/p_saveg.c index e557e494..d2a4393d 100644 --- a/src/p_saveg.c +++ b/src/p_saveg.c @@ -44,6 +44,7 @@ FILE *save_stream; int savegamelength; +boolean savegame_error; // Get the filename of a temporary file to write the savegame to. After // the file has been successfully saved, it will be renamed to the @@ -88,14 +89,31 @@ static byte saveg_read8(void) { byte result; - fread(&result, 1, 1, save_stream); + if (fread(&result, 1, 1, save_stream) < 1) + { + if (!savegame_error) + { + fprintf(stderr, "saveg_read8: Unexpected end of file while " + "reading save game\n"); + + savegame_error = true; + } + } return result; } static void saveg_write8(byte value) { - fwrite(&value, 1, 1, save_stream); + if (fwrite(&value, 1, 1, save_stream) < 1) + { + if (!savegame_error) + { + fprintf(stderr, "saveg_write8: Error while writing save game\n"); + + savegame_error = true; + } + } } static short saveg_read16(void) diff --git a/src/p_saveg.h b/src/p_saveg.h index 3a96cc3e..5488289c 100644 --- a/src/p_saveg.h +++ b/src/p_saveg.h @@ -64,6 +64,7 @@ void P_ArchiveSpecials (void); void P_UnArchiveSpecials (void); extern FILE *save_stream; +extern boolean savegame_error; #endif -- cgit v1.2.3 From da43e27597e40f5a298ed5c4ae785cc1b36510d4 Mon Sep 17 00:00:00 2001 From: Simon Howard Date: Sat, 1 May 2010 21:20:30 +0000 Subject: Silence printf(DEH_String(...)) warnings, by providing a DEH_printf function that checks the format string is a valid replacement. Also add DEH_fprintf and DEH_snprintf functions to use throughout the code to do similar checking. Subversion-branch: /trunk/chocolate-doom Subversion-revision: 1927 --- src/am_map.c | 6 +- src/d_main.c | 32 ++++---- src/d_net.c | 10 +-- src/deh_main.h | 8 ++ src/deh_text.c | 228 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/f_finale.c | 2 +- src/hu_stuff.c | 2 +- src/m_menu.c | 4 +- src/p_saveg.c | 2 +- src/p_setup.c | 4 +- src/st_stuff.c | 22 +++--- src/wi_stuff.c | 39 +++++----- 12 files changed, 297 insertions(+), 62 deletions(-) diff --git a/src/am_map.c b/src/am_map.c index 9a81d62c..4c58abdd 100644 --- a/src/am_map.c +++ b/src/am_map.c @@ -515,7 +515,7 @@ void AM_loadPics(void) for (i=0;i<10;i++) { - sprintf(namebuf, DEH_String("AMMNUM%d"), i); + DEH_snprintf(namebuf, 9, "AMMNUM%d", i); marknums[i] = W_CacheLumpName(namebuf, PU_STATIC); } @@ -528,7 +528,7 @@ void AM_unloadPics(void) for (i=0;i<10;i++) { - sprintf(namebuf, DEH_String("AMMNUM%d"), i); + DEH_snprintf(namebuf, 9, "AMMNUM%d", i); W_ReleaseLumpName(namebuf); } } @@ -1035,7 +1035,7 @@ AM_drawFline || fl->b.x < 0 || fl->b.x >= f_w || fl->b.y < 0 || fl->b.y >= f_h) { - fprintf(stderr, DEH_String("fuck %d \r"), fuck++); + DEH_fprintf(stderr, "fuck %d \r", fuck++); return; } diff --git a/src/d_main.c b/src/d_main.c index 3580c346..671c4e9c 100644 --- a/src/d_main.c +++ b/src/d_main.c @@ -819,7 +819,7 @@ void D_DoomMain (void) PrintBanner(PACKAGE_STRING); - printf (DEH_String("Z_Init: Init zone memory allocation daemon. \n")); + DEH_printf("Z_Init: Init zone memory allocation daemon. \n"); Z_Init (); #ifdef FEATURE_MULTIPLAYER @@ -936,7 +936,7 @@ void D_DoomMain (void) deathmatch = 2; if (devparm) - printf(DEH_String(D_DEVSTR)); + DEH_printf(D_DEVSTR); // find which dir to use for config files @@ -962,7 +962,7 @@ void D_DoomMain (void) scale = 10; if (scale > 400) scale = 400; - printf (DEH_String("turbo scale: %i%%\n"),scale); + DEH_printf("turbo scale: %i%%\n", scale); forwardmove[0] = forwardmove[0]*scale/100; forwardmove[1] = forwardmove[1]*scale/100; sidemove[0] = sidemove[0]*scale/100; @@ -970,14 +970,14 @@ void D_DoomMain (void) } // init subsystems - printf (DEH_String("V_Init: allocate screens.\n")); + DEH_printf("V_Init: allocate screens.\n"); V_Init (); - printf (DEH_String("M_LoadDefaults: Load system defaults.\n")); + DEH_printf("M_LoadDefaults: Load system defaults.\n"); M_ApplyPlatformDefaults(); M_LoadDefaults (); // load before initing other systems - printf (DEH_String("W_Init: Init WADfiles.\n")); + DEH_printf("W_Init: Init WADfiles.\n"); D_AddFile(iwadfile); #ifdef FEATURE_WAD_MERGE @@ -1322,8 +1322,8 @@ void D_DoomMain (void) if (p && p < myargc-1 && deathmatch) { - printf(DEH_String("Austin Virtual Gaming: Levels will end " - "after 20 minutes\n")); + DEH_printf("Austin Virtual Gaming: Levels will end " + "after 20 minutes\n"); timelimit = 20; } @@ -1437,16 +1437,16 @@ void D_DoomMain (void) PrintDehackedBanners(); - printf (DEH_String("M_Init: Init miscellaneous info.\n")); + DEH_printf("M_Init: Init miscellaneous info.\n"); M_Init (); - printf (DEH_String("R_Init: Init DOOM refresh daemon - ")); + DEH_printf("R_Init: Init DOOM refresh daemon - "); R_Init (); - printf (DEH_String("\nP_Init: Init Playloop state.\n")); + DEH_printf("\nP_Init: Init Playloop state.\n"); P_Init (); - printf (DEH_String("I_Init: Setting up machine state.\n")); + DEH_printf("I_Init: Setting up machine state.\n"); I_Init (); #ifdef FEATURE_MULTIPLAYER @@ -1454,18 +1454,18 @@ void D_DoomMain (void) NET_Init (); #endif - printf (DEH_String("S_Init: Setting up sound.\n")); + DEH_printf("S_Init: Setting up sound.\n"); S_Init (sfxVolume * 8, musicVolume * 8); - printf (DEH_String("D_CheckNetGame: Checking network game status.\n")); + DEH_printf("D_CheckNetGame: Checking network game status.\n"); D_CheckNetGame (); PrintGameVersion(); - printf (DEH_String("HU_Init: Setting up heads up display.\n")); + DEH_printf("HU_Init: Setting up heads up display.\n"); HU_Init (); - printf (DEH_String("ST_Init: Init status bar.\n")); + DEH_printf("ST_Init: Init status bar.\n"); ST_Init (); // If Doom II without a MAP01 lump, this is a store demo. diff --git a/src/d_net.c b/src/d_net.c index f5890769..faf47821 100644 --- a/src/d_net.c +++ b/src/d_net.c @@ -370,17 +370,17 @@ void D_CheckNetGame (void) ++num_players; } - printf (DEH_String("startskill %i deathmatch: %i startmap: %i startepisode: %i\n"), - startskill, deathmatch, startmap, startepisode); + DEH_printf("startskill %i deathmatch: %i startmap: %i startepisode: %i\n", + startskill, deathmatch, startmap, startepisode); - printf(DEH_String("player %i of %i (%i nodes)\n"), - consoleplayer+1, num_players, num_players); + DEH_printf("player %i of %i (%i nodes)\n", + consoleplayer+1, num_players, num_players); // Show players here; the server might have specified a time limit if (timelimit > 0) { - printf(DEH_String("Levels will end after %d minute"),timelimit); + DEH_printf("Levels will end after %d minute", timelimit); if (timelimit > 1) printf("s"); printf(".\n"); diff --git a/src/deh_main.h b/src/deh_main.h index 6afe07f3..9248e982 100644 --- a/src/deh_main.h +++ b/src/deh_main.h @@ -27,6 +27,8 @@ #ifndef DEH_MAIN_H #define DEH_MAIN_H +#include + #include "doomtype.h" #include "doomfeatures.h" #include "md5.h" @@ -52,10 +54,16 @@ void DEH_Checksum(md5_digest_t digest); #ifdef FEATURE_DEHACKED char *DEH_String(char *s); +void DEH_printf(char *fmt, ...); +void DEH_fprintf(FILE *fstream, char *fmt, ...); +void DEH_snprintf(char *buffer, size_t len, char *fmt, ...); #else #define DEH_String(x) (x) +#define DEH_printf printf +#define DEH_fprintf fprintf +#define DEH_snprintf snprintf #endif diff --git a/src/deh_text.c b/src/deh_text.c index 9e8cc79d..5c6446dc 100644 --- a/src/deh_text.c +++ b/src/deh_text.c @@ -24,6 +24,8 @@ // //----------------------------------------------------------------------------- +#include + #include "doomdef.h" #include "doomtype.h" @@ -251,3 +253,229 @@ deh_section_t deh_section_text = NULL, }; +typedef enum +{ + FORMAT_ARG_INVALID, + FORMAT_ARG_INT, + FORMAT_ARG_FLOAT, + FORMAT_ARG_CHAR, + FORMAT_ARG_STRING, + FORMAT_ARG_PTR, + FORMAT_ARG_SAVE_POS +} format_arg_t; + +// Get the type of a format argument. +// We can mix-and-match different format arguments as long as they +// are for the same data type. + +static format_arg_t FormatArgumentType(char c) +{ + switch (c) + { + case 'd': case 'i': case 'o': case 'u': case 'x': case 'X': + return FORMAT_ARG_INT; + + case 'e': case 'E': case 'f': case 'F': case 'g': case 'G': + case 'a': case 'A': + return FORMAT_ARG_FLOAT; + + case 'c': case 'C': + return FORMAT_ARG_CHAR; + + case 's': case 'S': + return FORMAT_ARG_STRING; + + case 'p': + return FORMAT_ARG_PTR; + + case 'n': + return FORMAT_ARG_SAVE_POS; + + default: + return FORMAT_ARG_INVALID; + } +} + +// Given the specified string, get the type of the first format +// string encountered. + +static format_arg_t NextFormatArgument(char **str) +{ + format_arg_t argtype; + + // Search for the '%' starting the next string. + + while (**str != '\0') + { + if (**str == '%') + { + ++*str; + + // Don't stop for double-%s. + + if (**str != '%') + { + break; + } + } + + ++*str; + } + + // Find the type of the format string. + + while (**str != '\0') + { + argtype = FormatArgumentType(**str); + + if (argtype != FORMAT_ARG_INVALID) + { + ++*str; + + return argtype; + } + + ++*str; + } + + // Stop searching, we have reached the end. + + *str = NULL; + + return FORMAT_ARG_INVALID; +} + +// Check if the specified argument type is a valid replacement for +// the original. + +static boolean ValidArgumentReplacement(format_arg_t original, + format_arg_t replacement) +{ + // In general, the original and replacement types should be + // identical. However, there are some cases where the replacement + // is valid and the types don't match. + + // Characters can be represented as ints. + + if (original == FORMAT_ARG_CHAR && replacement == FORMAT_ARG_INT) + { + return true; + } + + // Strings are pointers. + + if (original == FORMAT_ARG_STRING && replacement == FORMAT_ARG_PTR) + { + return true; + } + + return original == replacement; +} + +// Return true if the specified string contains no format arguments. + +static boolean ValidFormatReplacement(char *original, char *replacement) +{ + char *rover1; + char *rover2; + int argtype1, argtype2; + + // Check each argument in turn and compare types. + + rover1 = original; rover2 = replacement; + + for (;;) + { + argtype1 = NextFormatArgument(&rover1); + argtype2 = NextFormatArgument(&rover2); + + if (argtype2 == FORMAT_ARG_INVALID) + { + // No more arguments left to read from the replacement string. + + break; + } + else if (argtype1 == FORMAT_ARG_INVALID) + { + // Replacement string has more arguments than the original. + + return false; + } + else if (!ValidArgumentReplacement(argtype1, argtype2)) + { + // Not a valid replacement argument. + + return false; + } + } + + return true; +} + +// Get replacement format string, checking arguments. + +static char *FormatStringReplacement(char *s) +{ + char *repl; + + repl = DEH_String(s); + + if (!ValidFormatReplacement(s, repl)) + { + printf("WARNING: Unsafe dehacked replacement provided for " + "printf format string: %s\n", s); + + return s; + } + + return repl; +} + +// printf(), performing a replacement on the format string. + +void DEH_printf(char *fmt, ...) +{ + va_list args; + char *repl; + + repl = FormatStringReplacement(fmt); + + va_start(args, fmt); + + vprintf(repl, args); + + va_end(args); +} + +// fprintf(), performing a replacement on the format string. + +void DEH_fprintf(FILE *fstream, char *fmt, ...) +{ + va_list args; + char *repl; + + repl = FormatStringReplacement(fmt); + + va_start(args, fmt); + + vfprintf(fstream, repl, args); + + va_end(args); +} + +// snprintf(), performing a replacement on the format string. + +void DEH_snprintf(char *buffer, size_t len, char *fmt, ...) +{ + va_list args; + char *repl; + + repl = FormatStringReplacement(fmt); + + va_start(args, fmt); + + vsnprintf(buffer, len, repl, args); + + va_end(args); +} + diff --git a/src/f_finale.c b/src/f_finale.c index 1a3a85ca..3f0082cb 100644 --- a/src/f_finale.c +++ b/src/f_finale.c @@ -663,7 +663,7 @@ void F_BunnyScroll (void) laststage = stage; } - sprintf (name, DEH_String("END%i"), stage); + DEH_snprintf(name, 10, "END%i", stage); V_DrawPatch ((SCREENWIDTH-13*8)/2, (SCREENHEIGHT-8*8)/2,0, W_CacheLumpName (name,PU_CACHE)); } diff --git a/src/hu_stuff.c b/src/hu_stuff.c index 9f86c0b9..1381532d 100644 --- a/src/hu_stuff.c +++ b/src/hu_stuff.c @@ -345,7 +345,7 @@ void HU_Init(void) j = HU_FONTSTART; for (i=0;iepsd, i); + DEH_snprintf(name, 9, "WILV%d%d", wbs->epsd, i); callback(name, &lnames[i]); } @@ -1610,7 +1610,7 @@ static void WI_loadUnloadData(load_callback_t callback) // splat callback(DEH_String("WISPLAT"), &splat[0]); - + if (wbs->epsd < 3) { for (j=0;jepsd];j++) @@ -1619,17 +1619,16 @@ static void WI_loadUnloadData(load_callback_t callback) for (i=0;inanims;i++) { // MONDO HACK! - if (wbs->epsd != 1 || j != 8) + if (wbs->epsd != 1 || j != 8) { // animations - sprintf(name, DEH_String("WIA%d%.2d%.2d"), - wbs->epsd, j, i); + DEH_snprintf(name, 9, "WIA%d%.2d%.2d", wbs->epsd, j, i); callback(name, &a->p[i]); } else { // HACK ALERT! - a->p[i] = anims[1][4].p[i]; + a->p[i] = anims[1][4].p[i]; } } } @@ -1642,7 +1641,7 @@ static void WI_loadUnloadData(load_callback_t callback) for (i=0;i<10;i++) { // numbers 0-9 - sprintf(name, DEH_String("WINUM%d"), i); + DEH_snprintf(name, 9, "WINUM%d", i); callback(name, &num[i]); } @@ -1683,13 +1682,13 @@ static void WI_loadUnloadData(load_callback_t callback) callback(DEH_String("WICOLON"), &colon); // "time" - callback(DEH_String("WITIME"), &timepatch); + callback(DEH_String("WITIME"), &timepatch); // "sucks" - callback(DEH_String("WISUCKS"), &sucks); + callback(DEH_String("WISUCKS"), &sucks); // "par" - callback(DEH_String("WIPAR"), &par); + callback(DEH_String("WIPAR"), &par); // "killers" (vertical) callback(DEH_String("WIKILRS"), &killers); @@ -1698,16 +1697,16 @@ static void WI_loadUnloadData(load_callback_t callback) callback(DEH_String("WIVCTMS"), &victims); // "total" - callback(DEH_String("WIMSTT"), &total); + callback(DEH_String("WIMSTT"), &total); for (i=0 ; iepsd); + DEH_snprintf(bg_lumpname, 9, "WIMAP%d", wbs->epsd); } - + bg = W_CacheLumpName(bg_lumpname, PU_CACHE); V_DrawPatch(0, 0, 1, bg); } -- cgit v1.2.3 From 6787e90da50f6954d80512d172a985af4a6fc299 Mon Sep 17 00:00:00 2001 From: Simon Howard Date: Sat, 1 May 2010 21:47:26 +0000 Subject: Further sanity checking on use of strcpy() with dehacked string replacements. Subversion-branch: /trunk/chocolate-doom Subversion-revision: 1928 --- src/net_client.c | 3 ++- src/wi_stuff.c | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/net_client.c b/src/net_client.c index 0d6dd2fc..322869d7 100644 --- a/src/net_client.c +++ b/src/net_client.c @@ -193,7 +193,8 @@ static void NET_CL_PlayerQuitGame(player_t *player) // Do this the same way as Vanilla Doom does, to allow dehacked // replacements of this message - strcpy(exitmsg, DEH_String("Player 1 left the game")); + strncpy(exitmsg, DEH_String("Player 1 left the game"), sizeof(exitmsg)); + exitmsg[sizeof(exitmsg) - 1] = '\0'; exitmsg[7] += player - players; diff --git a/src/wi_stuff.c b/src/wi_stuff.c index e7eece07..e8fcab42 100644 --- a/src/wi_stuff.c +++ b/src/wi_stuff.c @@ -1749,11 +1749,13 @@ void WI_loadData(void) if (gamemode == commercial) { - strcpy(bg_lumpname, DEH_String("INTERPIC")); + strncpy(bg_lumpname, 9, DEH_String("INTERPIC")); + bg_lumpname[8] = '\0'; } else if (gamemode == retail && wbs->epsd == 3) { - strcpy(bg_lumpname, DEH_String("INTERPIC")); + strncpy(bg_lumpname, 9, DEH_String("INTERPIC")); + bg_lumpname[8] = '\0'; } else { -- cgit v1.2.3 From b3d5b80105dbc04b1e4c5999118a23b13aad1c5a Mon Sep 17 00:00:00 2001 From: Simon Howard Date: Mon, 3 May 2010 15:58:52 +0000 Subject: Update NEWS. Subversion-branch: /trunk/chocolate-doom Subversion-revision: 1929 --- NEWS | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/NEWS b/NEWS index 31baedf0..ea6ba572 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,27 @@ +1.4.0 (2010-??-??): + + The biggest change in this version is the addition of OPL + emulation. This emulates Vanilla Doom's MIDI playback when + using a Yamaha OPL synthesizer chip, as was found on + SoundBlaster compatible cards. + + A software OPL emulator is included as most modern computers do + not have a hardware OPL chip any more. If you do have one, you + can configure Chocolate Doom to use it; see README.OPL. + + The OPL playback feature is not yet perfect or 100% complete, + but is judged to be good enough for general use. If you find + music that does not play back properly, please report it as a + bug. + + Other changes: + * The REJECT overflow emulation code from PrBoom+ has been + imported. This fixes demo desync on some demos, although + others will still desync. + * Warnings are now generated for invalid dehacked replacements of + printf format strings. Some potential buffer overflows are + also checked. + 1.3.0 (2010-02-10): * Chocolate Doom now runs on Windows Mobile/Windows CE! -- cgit v1.2.3 From aa35a71bd05536f82ce23887f7e84decc7f850a7 Mon Sep 17 00:00:00 2001 From: Simon Howard Date: Mon, 3 May 2010 16:47:25 +0000 Subject: Oops. Subversion-branch: /trunk/chocolate-doom Subversion-revision: 1930 --- src/wi_stuff.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wi_stuff.c b/src/wi_stuff.c index e8fcab42..7c0ba312 100644 --- a/src/wi_stuff.c +++ b/src/wi_stuff.c @@ -1749,12 +1749,12 @@ void WI_loadData(void) if (gamemode == commercial) { - strncpy(bg_lumpname, 9, DEH_String("INTERPIC")); + strncpy(bg_lumpname, DEH_String("INTERPIC"), 9); bg_lumpname[8] = '\0'; } else if (gamemode == retail && wbs->epsd == 3) { - strncpy(bg_lumpname, 9, DEH_String("INTERPIC")); + strncpy(bg_lumpname, DEH_String("INTERPIC"), 9); bg_lumpname[8] = '\0'; } else -- cgit v1.2.3