Merge pull request #1041 from criezy/variadic-undefined

Fix undefined behaviour in variadic functions

14 files changed, 34 insertions, 25 deletions

diff --git a/engines/hugo/display.cpp b/engines/hugo/display.cpp
index 17627608bd..77df20eda5 100644
--- a/engines/hugo/display.cpp
+++ b/engines/hugo/display.cpp
@@ -332,7 +332,7 @@ int16 Screen::mergeLists(Rect *list, Rect *blist, const int16 len, int16 blen) {
  * Process the display list
  * Trailing args are int16 x,y,dx,dy for the D_ADD operation
  */
-void Screen::displayList(Dupdate update, ...) {
+void Screen::displayList(int update, ...) {
 	debugC(6, kDebugDisplay, "displayList()");
 
 	int16         blitLength = 0;                   // Length of blit list
diff --git a/engines/hugo/display.h b/engines/hugo/display.h
index 99fda0a638..33a0be8850 100644
--- a/engines/hugo/display.h
+++ b/engines/hugo/display.h
@@ -61,7 +61,7 @@ public:
 
 	void     displayBackground();
 	void     displayFrame(const int sx, const int sy, Seq *seq, const bool foreFl);
-	void     displayList(Dupdate update, ...);
+	void     displayList(int update, ...);
 	void     displayRect(const int16 x, const int16 y, const int16 dx, const int16 dy);
 	void     drawBoundaries();
 	void     drawRectangle(const bool filledFl, const int16 x1, const int16 y1, const int16 x2, const int16 y2, const int color);
diff --git a/engines/hugo/inventory.cpp b/engines/hugo/inventory.cpp
index 64609e6327..b638322847 100644
--- a/engines/hugo/inventory.cpp
+++ b/engines/hugo/inventory.cpp
@@ -139,8 +139,8 @@ void InventoryHandler::constructInventory(const int16 imageTotNumb, int displayN
  * Process required action for inventory
  * Returns objId under cursor (or -1) for INV_GET
  */
-int16 InventoryHandler::processInventory(const InvAct action, ...) {
-	debugC(1, kDebugInventory, "processInventory(InvAct action, ...)");
+int16 InventoryHandler::processInventory(const int action, ...) {
+	debugC(1, kDebugInventory, "processInventory(int action, ...)");
 
 	int16 imageNumb;                                // Total number of inventory items
 	int displayNumb;                                // Total number displayed/carried
diff --git a/engines/hugo/inventory.h b/engines/hugo/inventory.h
index a57bff4b58..61e9009559 100644
--- a/engines/hugo/inventory.h
+++ b/engines/hugo/inventory.h
@@ -49,7 +49,7 @@ public:
 
 	int16 findIconId(int16 objId);
 	void  loadInvent(Common::SeekableReadStream &in);
-	int16 processInventory(const InvAct action, ...);
+	int16 processInventory(const int action, ...);
 	void  runInventory();
 
 private:
diff --git a/engines/kyra/screen_lol.cpp b/engines/kyra/screen_lol.cpp
index 6dd6080923..2531cb4cb1 100644
--- a/engines/kyra/screen_lol.cpp
+++ b/engines/kyra/screen_lol.cpp
@@ -54,7 +54,7 @@ Screen_LoL::~Screen_LoL() {
 	delete[] _grayOverlay;
 }
 
-void Screen_LoL::fprintString(const char *format, int x, int y, uint8 col1, uint8 col2, uint16 flags, ...) {
+void Screen_LoL::fprintString(const char *format, int x, int y, uint8 col1, uint8 col2, uint flags, ...) {
 	if (!format)
 		return;
 
@@ -90,7 +90,7 @@ void Screen_LoL::fprintString(const char *format, int x, int y, uint8 col1, uint
 	printText(string, x, y, col1, col2);
 }
 
-void Screen_LoL::fprintStringIntro(const char *format, int x, int y, uint8 c1, uint8 c2, uint8 c3, uint16 flags, ...) {
+void Screen_LoL::fprintStringIntro(const char *format, int x, int y, uint8 c1, uint8 c2, uint8 c3, uint flags, ...) {
 	char buffer[400];
 
 	va_list args;
diff --git a/engines/kyra/screen_lol.h b/engines/kyra/screen_lol.h
index 91d663d34f..49c6f90168 100644
--- a/engines/kyra/screen_lol.h
+++ b/engines/kyra/screen_lol.h
@@ -36,8 +36,8 @@ public:
 	Screen_LoL(LoLEngine *vm, OSystem *system);
 	~Screen_LoL();
 
-	void fprintString(const char *format, int x, int y, uint8 col1, uint8 col2, uint16 flags, ...) GCC_PRINTF(2, 8);
-	void fprintStringIntro(const char *format, int x, int y, uint8 c1, uint8 c2, uint8 c3, uint16 flags, ...) GCC_PRINTF(2, 9);
+	void fprintString(const char *format, int x, int y, uint8 col1, uint8 col2, uint flags, ...) GCC_PRINTF(2, 8);
+	void fprintStringIntro(const char *format, int x, int y, uint8 c1, uint8 c2, uint8 c3, uint flags, ...) GCC_PRINTF(2, 9);
 
 	void drawShadedBox(int x1, int y1, int x2, int y2, int color1, int color2);
 
diff --git a/engines/lure/res_struct.cpp b/engines/lure/res_struct.cpp
index ec1a546d05..286489da82 100644
--- a/engines/lure/res_struct.cpp
+++ b/engines/lure/res_struct.cpp
@@ -827,9 +827,13 @@ void SequenceDelayList::loadFromStream(Common::ReadStream *stream) {
 
 // The following classes hold the NPC schedules
 
-CharacterScheduleEntry::CharacterScheduleEntry(Action theAction, ...) {
+// The following function should really take a Action parameter, but the
+// behaviour is undefined if the last argument of a variadic function
+// undergoes default argument promotion, which might be the case for enum
+// types.
+CharacterScheduleEntry::CharacterScheduleEntry(int theAction, ...) {
 	_parent = NULL;
-	_action = theAction;
+	_action = (Action)theAction;
 
 	va_list u_Arg;
 	va_start(u_Arg, theAction);
@@ -870,8 +874,10 @@ uint16 CharacterScheduleEntry::param(int index) {
 	return _params[index];
 }
 
-void CharacterScheduleEntry::setDetails(Action theAction, ...) {
-	_action = theAction;
+// The parameter to this function should really be an Action.
+// But... (see comment above for CharacterScheduleEntry(int, ...))
+void CharacterScheduleEntry::setDetails(int theAction, ...) {
+	_action = (Action)theAction;
 	_numParams = actionNumParams[_action];
 
 	va_list list;
diff --git a/engines/lure/res_struct.h b/engines/lure/res_struct.h
index 685c55ab13..a8a5e5aca8 100644
--- a/engines/lure/res_struct.h
+++ b/engines/lure/res_struct.h
@@ -404,7 +404,7 @@ private:
 	int _numParams;
 public:
 	CharacterScheduleEntry() { _action = NONE; _parent = NULL; }
-	CharacterScheduleEntry(Action theAction, ...);
+	CharacterScheduleEntry(int theAction, ...);
 	CharacterScheduleEntry(CharacterScheduleSet *parentSet,
 		CharacterScheduleResource *&rec);
 	CharacterScheduleEntry(CharacterScheduleEntry *src);
@@ -412,7 +412,7 @@ public:
 	Action action() { return _action; }
 	int numParams() { return _numParams; }
 	uint16 param(int index);
-	void setDetails(Action theAction, ...);
+	void setDetails(int theAction, ...);
 	void setDetails2(Action theAction, int numParamEntries, uint16 *paramList);
 	CharacterScheduleEntry *next();
 	CharacterScheduleSet *parent() { return _parent; }
diff --git a/engines/mohawk/riven_scripts.cpp b/engines/mohawk/riven_scripts.cpp
index cb040ee4a0..18a3597086 100644
--- a/engines/mohawk/riven_scripts.cpp
+++ b/engines/mohawk/riven_scripts.cpp
@@ -147,13 +147,13 @@ void RivenScriptManager::runQueuedScripts() {
 	_runningQueuedScripts = false;
 }
 
-RivenScriptPtr RivenScriptManager::createScriptFromData(uint16 commandCount, ...) {
+RivenScriptPtr RivenScriptManager::createScriptFromData(uint commandCount, ...) {
 	va_list args;
 	va_start(args, commandCount);
 
 	// Build a script from the variadic arguments
 	Common::MemoryWriteStreamDynamic writeStream = Common::MemoryWriteStreamDynamic(DisposeAfterUse::YES);
-	writeStream.writeUint16BE(commandCount);
+	writeStream.writeUint16BE((uint16)commandCount);
 
 	for (uint i = 0; i < commandCount; i++) {
 		uint16 command = va_arg(args, int);
diff --git a/engines/mohawk/riven_scripts.h b/engines/mohawk/riven_scripts.h
index 25cf363450..423434fe59 100644
--- a/engines/mohawk/riven_scripts.h
+++ b/engines/mohawk/riven_scripts.h
@@ -179,7 +179,7 @@ public:
 	RivenScriptPtr readScriptFromData(uint16 *data, uint16 size);
 
 	/** Create a script from the caller provided arguments containing raw data */
-	RivenScriptPtr createScriptFromData(uint16 commandCount, ...);
+	RivenScriptPtr createScriptFromData(uint commandCount, ...);
 
 	/**
 	 * Create a script with a single user provided command
diff --git a/engines/tony/mpal/mpal.cpp b/engines/tony/mpal/mpal.cpp
index 9172843781..7319509624 100644
--- a/engines/tony/mpal/mpal.cpp
+++ b/engines/tony/mpal/mpal.cpp
@@ -1548,7 +1548,7 @@ void mpalFree() {
  * @remarks		This is the specialized version of the original single mpalQuery
  * method that returns numeric results.
  */
-uint32 mpalQueryDWORD(uint16 wQueryType, ...) {
+uint32 mpalQueryDWORD(uint wQueryType, ...) {
 	Common::String buf;
 	uint32 dwRet = 0;
 
@@ -1744,7 +1744,7 @@ uint32 mpalQueryDWORD(uint16 wQueryType, ...) {
  * @remarks		This is the specialized version of the original single mpalQuery
  * method that returns a pointer or handle.
  */
-MpalHandle mpalQueryHANDLE(uint16 wQueryType, ...) {
+MpalHandle mpalQueryHANDLE(uint wQueryType, ...) {
 	Common::String buf;
 	va_list v;
 	va_start(v, wQueryType);
diff --git a/engines/tony/mpal/mpal.h b/engines/tony/mpal/mpal.h
index af24c46697..56448a554d 100644
--- a/engines/tony/mpal/mpal.h
+++ b/engines/tony/mpal/mpal.h
@@ -394,7 +394,7 @@ void mpalFree();
  * @remarks		This is the specialized version of the original single mpalQuery
  * method that returns numeric results.
  */
-uint32 mpalQueryDWORD(uint16 wQueryType, ...);
+uint32 mpalQueryDWORD(uint wQueryType, ...);
 
 /**
  * This is a general function to communicate with the library, to request information
@@ -405,7 +405,7 @@ uint32 mpalQueryDWORD(uint16 wQueryType, ...);
  * @remarks		This is the specialized version of the original single mpalQuery
  * method that returns a pointer or handle.
  */
-MpalHandle mpalQueryHANDLE(uint16 wQueryType, ...);
+MpalHandle mpalQueryHANDLE(uint wQueryType, ...);
 
 /**
  * This is a general function to communicate with the library, to request information
diff --git a/engines/tsage/core.cpp b/engines/tsage/core.cpp
index 985d16b031..a3ed3abb5b 100644
--- a/engines/tsage/core.cpp
+++ b/engines/tsage/core.cpp
@@ -2358,8 +2358,11 @@ int SceneObject::checkRegion(const Common::Point &pt) {
 	return regionIndex;
 }
 
-void SceneObject::animate(AnimateMode animMode, ...) {
-	_animateMode = animMode;
+// The parameter to the function below should really be an AnimateMode value.
+// However passing an enum type as last argument of a variadic function may
+// result in undefined behaviour.
+void SceneObject::animate(int animMode, ...) {
+	_animateMode = (AnimateMode)animMode;
 	_updateStartFrame = g_globals->_events.getFrameNumber();
 	if (_numFrames)
 		_updateStartFrame += 60 / _numFrames;
diff --git a/engines/tsage/core.h b/engines/tsage/core.h
index 8b1deadaeb..1cee491b4c 100644
--- a/engines/tsage/core.h
+++ b/engines/tsage/core.h
@@ -577,7 +577,7 @@ public:
 	void getHorizBounds();
 	int getRegionIndex();
 	int checkRegion(const Common::Point &pt);
-	void animate(AnimateMode animMode, ...);
+	void animate(int animMode, ...);
 	void checkAngle(const SceneObject *obj);
 	void checkAngle(const Common::Point &pt);
 	void hide();