Use scanf and printf compile time format string type checking; fix 64 bit bugs

This commit is contained in:
Scott Ludwig 2016-01-10 13:14:36 -08:00
parent ac148a79f1
commit de950ccc46
16 changed files with 89 additions and 70 deletions

View File

@ -62,6 +62,13 @@
#define RLOGX() while(false) base::EmptyLog()
#endif
// Format string compile time check
#if defined(__GNUC__) || defined(__clang__)
#define printfFormat12 __attribute__((format(printf, 1, 2)))
#else
#define printfFormat12
#endif
// Log string formatting class (for legacy reasons called Log)
#if defined(CONSOLE_LOGGING)
@ -73,7 +80,7 @@
namespace base {
class Log {
public:
static std::string Format(const char *format, ...) {
static std::string Format(const char *format, ...) printfFormat12 {
va_list va;
va_start(va, format);
std::string str = vFormat(format, va);
@ -94,7 +101,7 @@ private:
namespace base {
class Log {
public:
static int Format(const char *format, ...) { return 0; }
static int Format(const char *format, ...) printfFormat12 { return 0; }
};
} // namespace base

View File

@ -22,9 +22,9 @@ void MessageHandler::Dispose() {
if (!disposed_) {
disposed_ = true;
thread_.Dispose(this);
LOG() << base::Log::Format("0x%08lx ", this) << "disposing";
LOG() << base::Log::Format("0x%p ", this) << "disposing";
} else {
LOG() << base::Log::Format("0x%08lx ", this)
LOG() << base::Log::Format("0x%p ", this)
<< "tried to dispose but already disposed";
}
}

View File

@ -65,7 +65,7 @@ bool Level::LoadLevelInfo(const char *pszLevelName, IniReader *piniLoaded)
// Get revision #
if (pini->GetPropertyValue("General", "Revision", "%ld", &m_dwRevision) == 0)
if (pini->GetPropertyValue("General", "Revision", "%d", &m_dwRevision) == 0)
m_dwRevision = 0;
// Can't run if the level version is newer than what the game supports
@ -138,7 +138,7 @@ bool Level::LoadSideInfo(IniReader *pini, char *pszSideName, SideInfo *psidi)
// Include some good defaults in case this level contains no SideInfo for this side
psidi->nInitialCredits = 5000;
pini->GetPropertyValue(pszSideName, "InitialCredits", "%ld", &psidi->nInitialCredits);
pini->GetPropertyValue(pszSideName, "InitialCredits", "%d", &psidi->nInitialCredits);
int tx = 0;
int ty = 0;

View File

@ -417,7 +417,7 @@ void MobileUnitGob::PerformAction(char *szAction)
case knHuntEnemiesUnitAction:
{
UnitMask um;
if (IniScanf(szAction, "%*d ,%ld", &um) == 0) {
if (IniScanf(szAction, "%*d ,%d", &um) == 0) {
Assert(false);
return;
}

View File

@ -110,7 +110,7 @@ bool UnitGob::InitClass(UnitConsts *puntc, IniReader *pini)
if (pini->GetPropertyValue(szTemplate, "SortOffset", "%d", &puntc->wdySortOffset) != 1)
return false;
puntc->wdySortOffset *= 16; // HACK: so we can keep existing GobTemplate sort offsets
if (pini->GetPropertyValue(szTemplate, "Animation", "%s", &puntc->szAniName) != 1)
if (pini->GetPropertyValue(szTemplate, "Animation", "%s", puntc->szAniName) != 1)
return false;
if (pini->GetPropertyValue(szTemplate, "Name", "%s", puntc->szName) != 1) {
Assert("Struct/Unit must have 'Name' property");

View File

@ -128,7 +128,7 @@ void GameMain(char *pszCmds)
// -l %s (level: level.lvl)
} else if (strcmp(szT, "-l") == 0) {
c = IniScanf(&pszCmds[nchAbs], "%s", szLevel, &nchRel);
c = IniScanf(&pszCmds[nchAbs], "%s%+", szLevel, &nchRel);
nchAbs += nchRel;
if (c == 0)
return;

View File

@ -381,7 +381,7 @@ bool CheckBoxControl::Init(Form *pfrm, IniReader *pini, FindProp *pfind)
char szChecked[64];
szChecked[0] = 0;
int cArgs = pini->GetPropertyValue(pfind, "%*d (%*d %*d %*d %*d) \"%s\" %d %s",
m_szLabel, &m_ifnt, &szChecked);
m_szLabel, &m_ifnt, szChecked);
if (cArgs == 2)
return Init(m_szLabel, m_ifnt, false);
if (cArgs != 3)

View File

@ -76,7 +76,7 @@ bool TriggerMgr::Init(IniReader *pini)
// Remember per-side assignments and indexes
int cArgs = pini->GetPropertyValue(&find, "%s", &szProp);
int cArgs = pini->GetPropertyValue(&find, "%s", szProp);
if (cArgs != 1)
return false;
if (!AssignTriggerSides(ntgr, szProp))
@ -681,7 +681,7 @@ bool ParseUnitMask(char **ppsz, UnitMask *pum)
{
*pum = 0;
int nch = 0;
int cArgs = IniScanf(*ppsz, "%ld,%+", pum, &nch);
int cArgs = IniScanf(*ppsz, "%d,%+", pum, &nch);
*ppsz += nch;
return cArgs >= 1;
}

View File

@ -6,7 +6,7 @@
namespace wi {
const dword kdwProtocolCurrent = 0x485407d4;
const dword kdwProtocolCurrent = 0x485407d5;
const byte XMSG_NONE = 0x00;
const byte XMSG_HANDSHAKE = 0x81;

View File

@ -31,15 +31,6 @@ namespace wi {
#endif
#define IsDigit(ch) ((ch) >= '0' && (ch) <= '9')
int IniScanf(char *pszBuff, char *pszFmt, ...)
{
va_list va;
va_start(va, pszFmt);
int c = VIniScanf(pszBuff, pszFmt, va);
va_end(va);
return c;
}
int VIniScanf(char *pszBuff, char *pszFmt, va_list va)
{
bool fLong;
@ -253,6 +244,15 @@ int VIniScanf(char *pszBuff, char *pszFmt, va_list va)
return cArgs;
}
int IniScanf(char *pszBuff, char *pszFmt, ...)
{
va_list va;
va_start(va, pszFmt);
int c = VIniScanf(pszBuff, pszFmt, va);
va_end(va);
return c;
}
IniReader *LoadIniFile(PackFileReader& pak, const char *pszFn)
{
IniReader *pini = new IniReader(pak);

View File

@ -5,6 +5,18 @@
#include "mpshared/packfile.h"
#include <stdarg.h>
// Don't leave this on all the time because IniScanf uses
// some non-standard format strings.
#if 0 // defined(__GNUC__) || defined(__clang__)
#define scanfFormat23 __attribute__((format(scanf, 2, 3)))
#define scanfFormat34 __attribute__((format(scanf, 3, 4)))
#define scanfFormat45 __attribute__((format(scanf, 4, 5)))
#else
#define scanfFormat23
#define scanfFormat34
#define scanfFormat45
#endif
namespace wi {
struct IniSection // sec
@ -45,9 +57,9 @@ public:
bool Init(const char *psz);
bool GetPropertyValue(char *pszSec, char *pszProp, char *pszValue, int cbValue);
int GetPropertyValue(char *pszSec, char *pszProp, char *pszFmt, ...);
bool FindNextProperty(FindProp *pfind, char *pszSec, char *pszProp, int cbProp);
int GetPropertyValue(FindProp *pfind, char *pszFmt, ...);
int GetPropertyValue(char *pszSec, char *pszProp, char *pszFmt, ...) scanfFormat45;
int GetPropertyValue(FindProp *pfind, char *pszFmt, ...) scanfFormat34;
private:
IniSection *FindSection(char *pszSec);
@ -58,9 +70,9 @@ private:
IniSection *m_psecFirst;
FileMap m_fmap;
};
IniReader *LoadIniFile(PackFileReader& pak, const char *pszFn);
int IniScanf(char *pszBuff, char *pszFmt, ...);
int VIniScanf(char *pszBuff, char *pszFmt, va_list va);
int IniScanf(char *pszBuff, char *pszFmt, ...) scanfFormat23;
} // namespace wi;

View File

@ -66,7 +66,7 @@ bool XPump::Send(base::ByteBuffer *pbb) {
#ifdef LOGGING
base::ByteBuffer *pbbT = pbb->Clone();
XMsg *pmsg = XMsgFromBuffer(*pbbT, pbbT->Length());
LOG() << base::Log::Format("0x%08lx", notify_) << ": " << pmsg->ToString()
LOG() << base::Log::Format("0x%p", notify_) << ": " << pmsg->ToString()
<< ", " << pbb->Length() << " bytes";
delete pmsg;
delete pbbT;
@ -155,7 +155,7 @@ bool XPump::Dispatch() {
}
#ifdef LOGGING
LOG() << base::Log::Format("0x%08lx", notify_) << ": "
LOG() << base::Log::Format("0x%p", notify_) << ": "
<< pmsg->ToString() << ", " << cbMsg << " bytes.";
#endif

View File

@ -29,14 +29,14 @@ Endpoint::Endpoint(Server& server, base::Socket *socket, dword id,
}
Endpoint::~Endpoint() {
LOG() << base::Log::Format("0x%08lx", this);
LOG() << base::Log::Format("0x%p", this);
SignalOnDelete(this);
delete name_;
}
void Endpoint::SetState(State state)
{
LOG() << base::Log::Format("0x%08lx ", this)
LOG() << base::Log::Format("0x%p ", this)
<< "From: " << EsLabels.Find(state_)
<< " To: " << EsLabels.Find(state);
state_ = state;
@ -48,13 +48,13 @@ bool Endpoint::CheckState(State state0, State state1)
Assert(state0 == state_ || state1 == state_);
#endif
if (state0 != state_ && state1 != state_) {
LOG() << base::Log::Format("0x%08lx ", this)
LOG() << base::Log::Format("0x%p ", this)
<< "Warning! Current: " << EsLabels.Find(state_);
if (state1 == (State)-1) {
LOG() << base::Log::Format("0x%08lx ", this)
LOG() << base::Log::Format("0x%p ", this)
<< " Expected: " << EsLabels.Find(state0);
} else {
LOG() << base::Log::Format("0x%08lx ", this)
LOG() << base::Log::Format("0x%p ", this)
<< " Expected: " << EsLabels.Find(state0)
<< " or " << EsLabels.Find(state1);
}
@ -76,7 +76,7 @@ void Endpoint::OnHandshake(dword clientid, dword protocolid) {
// The server and client need to support the same protocol.
if (protocolid != kdwProtocolCurrent) {
xpump_.Send(XMsgHandshakeResult::ToBuffer(knHandshakeResultFail, 0));
RLOG() << base::Log::Format("0x%08lx ", this)
RLOG() << base::Log::Format("0x%p ", this)
<< "wrong protocolid: " << protocolid
<< " clientid: " << clientid;
return;
@ -84,7 +84,7 @@ void Endpoint::OnHandshake(dword clientid, dword protocolid) {
if (clientid != kdwClientID) {
xpump_.Send(XMsgHandshakeResult::ToBuffer(knHandshakeResultFail, 0));
RLOG() << base::Log::Format("0x%08lx ", this)
RLOG() << base::Log::Format("0x%p ", this)
<< "wrong clientid: " << clientid;
return;
}
@ -92,7 +92,7 @@ void Endpoint::OnHandshake(dword clientid, dword protocolid) {
if (serverfull_) {
xpump_.Send(XMsgHandshakeResult::ToBuffer(knHandshakeResultServerFull,
0));
RLOG() << base::Log::Format("0x%08lx ", this) << "server is full.";
RLOG() << base::Log::Format("0x%p ", this) << "server is full.";
return;
}
@ -405,7 +405,7 @@ void Endpoint::OnRoomCreateGame(const GameParams& params) {
if (!ValidateGameParams(params)) {
xpump_.Send(XMsgRoomCreateGameResult::ToBuffer(0,
knRoomCreateGameResultFail, NULL));
RLOG() << base::Log::Format("0x%08lx ", this)
RLOG() << base::Log::Format("0x%p ", this)
<< "game params invalid";
#ifdef RELEASE_LOGGING
LogGameParams(params);
@ -421,7 +421,7 @@ void Endpoint::OnRoomCreateGame(const GameParams& params) {
// Don't know about this pack at all
xpump_.Send(XMsgRoomCreateGameResult::ToBuffer(0,
knRoomCreateGameResultUnknownMissionPack, NULL));
RLOG() << base::Log::Format("0x%08lx ", this)
RLOG() << base::Log::Format("0x%p ", this)
<< "packid not found";
#ifdef RELEASE_LOGGING
LogGameParams(params);
@ -436,7 +436,7 @@ void Endpoint::OnRoomCreateGame(const GameParams& params) {
// Know about this pack, but client has wrong version
xpump_.Send(XMsgRoomCreateGameResult::ToBuffer(0,
knRoomCreateGameResultUpgradeMissionPack, &packidUpgrade));
LOG() << base::Log::Format("0x%08lx ", this)
LOG() << base::Log::Format("0x%p ", this)
<< "client needs packid upgrade";
#ifdef LOGGING
LogGameParams(params);
@ -449,7 +449,7 @@ void Endpoint::OnRoomCreateGame(const GameParams& params) {
if (!room->CanAddGame(this)) {
xpump_.Send(XMsgRoomCreateGameResult::ToBuffer(0,
knRoomCreateGameResultRoomFull, NULL));
RLOG() << base::Log::Format("0x%08lx ", this)
RLOG() << base::Log::Format("0x%p ", this)
<< "room full: " << room->name();
return;
}
@ -1332,7 +1332,7 @@ void Endpoint::OnGameLeave() {
if (game_ == NULL) {
xpump_.Send(XMsgGameLeaveResult::ToBuffer(
knGameLeaveResultNotFound));
LOG() << base::Log::Format("0x%08lx ", this)
LOG() << base::Log::Format("0x%p ", this)
<< "No game to disconnect from. "
<< "Can happen when the server disconnects first";
return;
@ -1341,7 +1341,7 @@ void Endpoint::OnGameLeave() {
if (!CheckState(ES_GAME)) {
xpump_.Send(XMsgGameLeaveResult::ToBuffer(
knGameLeaveResultFail));
LOG() << base::Log::Format("0x%08lx ", this)
LOG() << base::Log::Format("0x%p ", this)
<< "Not in ES_GAME state.";
return;
}
@ -1364,14 +1364,14 @@ void Endpoint::OnGameNetMessage(NetMessage **ppnm) {
// Check game_ for NULL first, before state_ is checked,
// since the server can force game_ to NULL legally.
if (game_ == NULL) {
LOG() << base::Log::Format("0x%08lx ", this)
LOG() << base::Log::Format("0x%p ", this)
<< "No game for NetMessage. "
<< "Can happen when the server disconnects first";
return;
}
if (!CheckState(ES_GAME)) {
LOG() << base::Log::Format("0x%08lx ", this) << "Not in ES_GAME!";
LOG() << base::Log::Format("0x%p ", this) << "Not in ES_GAME!";
return;
}
game_->OnNetMessage(this, *ppnm);
@ -1384,7 +1384,7 @@ void Endpoint::OnGameDelete(Game *game) {
}
void Endpoint::DropGame(Game *game, int reason) {
LOG() << base::Log::Format("0x%08lx ", this)
LOG() << base::Log::Format("0x%p ", this)
<< "Dropping game, reason: " << reason;
Assert(game == game_);
@ -1399,17 +1399,17 @@ void Endpoint::DropGame(Game *game, int reason) {
}
void Endpoint::OnError(int error) {
LOG() << base::Log::Format("0x%08lx ", this) << error;
LOG() << base::Log::Format("0x%p ", this) << error;
Dispose();
}
void Endpoint::OnClose(int error) {
LOG() << base::Log::Format("0x%08lx ", this) << error;
LOG() << base::Log::Format("0x%p ", this) << error;
Dispose();
}
void Endpoint::OnCloseOk() {
LOG() << base::Log::Format("0x%08lx ", this);
LOG() << base::Log::Format("0x%p ", this);
Dispose();
}
@ -1420,7 +1420,7 @@ void Endpoint::OnHeartbeat() {
}
if (!echo_) {
LOG() << base::Log::Format("0x%08lx ", this)
LOG() << base::Log::Format("0x%p ", this)
<< "client ping timeout";
#ifndef DEV_BUILD
// When a user brings up audio controls (double click home), or

View File

@ -38,7 +38,7 @@ Game::Game(Endpoint *endpoint, const GameParams& params, const LevelInfo& info,
}
Game::~Game() {
LOG() << base::Log::Format("0x%08lx", this);
LOG() << base::Log::Format("0x%p", this);
if (advertiser_ != NULL) {
advertiser_->SignalOnDelete.disconnect(this);
advertiser_ = NULL;
@ -131,13 +131,13 @@ void Game::AddPlayer(Endpoint *endpoint) {
}
void Game::RemovePlayer(Endpoint *endpoint, int reason, bool disconnect) {
LOG() << base::Log::Format("0x%08lx ", endpoint)
LOG() << base::Log::Format("0x%p ", endpoint)
<< "endpoint disconnecting, reason:" << reason;
if (endpoint->id() == advertiserId_) {
// If this game hasn't started yet, dispose the game
if (!playing_) {
LOG() << base::Log::Format("0x%08lx ", this)
LOG() << base::Log::Format("0x%p ", this)
<< "advertiser leaving game, auto disposing!";
Dispose();
}
@ -183,7 +183,7 @@ void Game::RemovePlayer(Endpoint *endpoint, int reason, bool disconnect) {
endpoint->SignalOnDelete.disconnect(this);
}
if (connected_.size() == 0) {
LOG() << base::Log::Format("0x%08lx ", this)
LOG() << base::Log::Format("0x%p ", this)
<< "no endpoints connected - auto disposing!";
Dispose();
}
@ -224,7 +224,7 @@ void Game::OnNetMessage(Endpoint *endpoint, NetMessage *pnm) {
break;
case knmidCsConnect:
RLOG() << base::Log::Format("0x%08lx ", endpoint)
RLOG() << base::Log::Format("0x%p ", endpoint)
<< "Received knmidCsConnect unexpectedly!";
break;
@ -261,7 +261,7 @@ void Game::OnNetMessage(Endpoint *endpoint, NetMessage *pnm) {
void Game::OnPlayerJoin(Endpoint *endpoint, PlayerJoinNetMessage *ppjnm) {
// If already playing, this message means nothing
if (playing_) {
RLOG() << base::Log::Format("0x%08lx ", endpoint)
RLOG() << base::Log::Format("0x%p ", endpoint)
<< "received PlayerJoin while playing.";
NetMessage nmsg(knmidScCantAcceptMoreConnections);
endpoint->xpump().Send(XMsgGameNetMessage::ToBuffer(&nmsg));
@ -307,7 +307,7 @@ void Game::OnPlayerJoin(Endpoint *endpoint, PlayerJoinNetMessage *ppjnm) {
void Game::OnClientReady(Endpoint *endpoint, NetMessage *pnm) {
// If already playing, this message means nothing
if (playing_) {
RLOG() << base::Log::Format("0x%08lx ", endpoint)
RLOG() << base::Log::Format("0x%p ", endpoint)
<< "received ClientReady while playing.";
return;
}
@ -315,7 +315,7 @@ void Game::OnClientReady(Endpoint *endpoint, NetMessage *pnm) {
// Update this player's state in the master list and let all players know
Player *pplr = playerMgr_.GetPlayer(endpoint);
if (pplr == NULL) {
RLOG() << base::Log::Format("0x%08lx ", endpoint)
RLOG() << base::Log::Format("0x%p ", endpoint)
<< "has no player.";
return;
}
@ -330,7 +330,7 @@ void Game::OnRequestBeginGame(Endpoint *endpoint, NetMessage *pnm) {
NetMessage nmFail(knmidScBeginGameFail);
if (playing_) {
endpoint->xpump().Send(XMsgGameNetMessage::ToBuffer(&nmFail));
RLOG() << base::Log::Format("0x%08lx ", endpoint)
RLOG() << base::Log::Format("0x%p ", endpoint)
<< "received RequestBeginGame while playing.";
return;
}
@ -620,7 +620,7 @@ void Game::OnWinStats(Endpoint *endpoint, WinStatsNetMessage *pnm) {
LOG() << endpoint->name();
Player *pplrEndpoint = playerMgr_.GetPlayer(endpoint);
if (pplrEndpoint == NULL) {
LOG() << base::Log::Format("0x%08lx ", endpoint)
LOG() << base::Log::Format("0x%p ", endpoint)
<< "Could not find player!";
return;
}
@ -635,11 +635,11 @@ void Game::OnWinStats(Endpoint *endpoint, WinStatsNetMessage *pnm) {
// as possible, so clever updating is required.
if (!ValidateWinStats(endpoint, pnm)) {
#ifdef DEBUG_LOGGING
LOG() << base::Log::Format("0x%08lx ", endpoint)
LOG() << base::Log::Format("0x%p ", endpoint)
<< "WinStats are invalid! " << PszFromNetMessage(pnm);
#endif
#ifdef RELEASE_LOGGING
RLOG() << base::Log::Format("0x%08lx ", endpoint)
RLOG() << base::Log::Format("0x%p ", endpoint)
<< "WinStats are invalid! username: " << endpoint->name()
<< " ip address: "
<< endpoint->xpump().socket()->GetRemoteAddress().ToString();
@ -686,12 +686,12 @@ void Game::OnWinStats(Endpoint *endpoint, WinStatsNetMessage *pnm) {
}
void Game::OnChallengeWin(Endpoint *endpoint) {
LOG() << base::Log::Format("0x%08lx ", endpoint)
LOG() << base::Log::Format("0x%p ", endpoint)
<< endpoint->name() << " challenges the win!";
Player *pplr = playerMgr_.GetPlayer(endpoint);
if (pplr == NULL) {
LOG() << base::Log::Format("0x%08lx ", endpoint)
LOG() << base::Log::Format("0x%p ", endpoint)
<< "Could not find player!";
return;
}
@ -728,7 +728,7 @@ void Game::SendClientCommands() {
int cbUnm = sizeof(UpdateNetMessage) + ((cmsg - 1) * sizeof(Message));
UpdateNetMessage *punm = (UpdateNetMessage *)new byte[cbUnm];
if (punm == NULL) {
LOG() << base::Log::Format("0x%08lx ", this)
LOG() << base::Log::Format("0x%p ", this)
<< "Allocation Error!";
Dispose();
return;
@ -780,7 +780,7 @@ void Game::OnKillLaggingPlayer(Endpoint *endpoint,
void Game::OnClientCommands(Endpoint *endpoint, ClientCommandsNetMessage *pnm) {
// If not yet playing, this message means nothing
if (!playing_) {
RLOG() << base::Log::Format("0x%08lx ", endpoint)
RLOG() << base::Log::Format("0x%p ", endpoint)
<< "ClientCommands received while not in game";
return;
}
@ -791,7 +791,7 @@ void Game::OnClientCommands(Endpoint *endpoint, ClientCommandsNetMessage *pnm) {
cmds_.push_back(pnm->amsgCommands[i]);
}
LOG() << base::Log::Format("0x%08lx ccmds:%d", endpoint, pnm->cmsgCommands);
LOG() << base::Log::Format("0x%p ccmds:%d", endpoint, pnm->cmsgCommands);
// TODO: Validation
}
@ -804,14 +804,14 @@ void Game::OnUpdateResult(Endpoint *endpoint, UpdateResultNetMessage *pnm) {
// If not yet playing, this message means nothing
if (!playing_) {
RLOG() << base::Log::Format("0x%08lx ", endpoint)
RLOG() << base::Log::Format("0x%p ", endpoint)
<< "UpdateResult received while not in game";
return;
}
Player *pplr = playerMgr_.GetPlayer(endpoint);
if (pplr == NULL) {
LOG() << base::Log::Format("0x%08lx ", endpoint)
LOG() << base::Log::Format("0x%p ", endpoint)
<< "Could not find player!";
return;
}

View File

@ -36,7 +36,7 @@ Room::Room(Server *server, Endpoint *creator, const char *name,
}
Room::~Room() {
LOG() << base::Log::Format("0x%08lx", this);
LOG() << base::Log::Format("0x%p", this);
Assert(endpointmap_.size() == 0);
SignalOnDelete(this);
delete creator_;

View File

@ -11,7 +11,7 @@ JsonObject::~JsonObject() {
#ifdef RELEASE_LOGGING
std::string JsonObject::ToString() const {
return base::Log::Format("JsonObject <0x%08lx>", this);
return base::Log::Format("JsonObject <0x%p>", this);
}
#endif