[Scummvm-git-logs] scummvm master -> 5964c36ab46d5ec0c94663cf8b78934d62109a86

lephilousophe noreply at scummvm.org
Fri Jul 14 09:10:34 UTC 2023


This automated email contains information about 6 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
6a1abd09f1 COMMON: Make sure coroutine Process parameter is well aligned
a831accde7 TONY: Fix various types discrepancies
e7ee27e99a TONY: Fix MemoryManager data alignment
1f1988f5ed TONY: Properly align Expressions
76fd10dafe TONY: Fix _num type discrepancy
5964c36ab4 COMMON: Don't use a pointer to T in UnalignedPtr


Commit: 6a1abd09f1959b1a2123832128f85f06d0b91b54
    https://github.com/scummvm/scummvm/commit/6a1abd09f1959b1a2123832128f85f06d0b91b54
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2023-07-14T11:10:28+02:00

Commit Message:
COMMON: Make sure coroutine Process parameter is well aligned

Changed paths:
    common/coroutines.h


diff --git a/common/coroutines.h b/common/coroutines.h
index 47c2e2635bd..1861ee53f88 100644
--- a/common/coroutines.h
+++ b/common/coroutines.h
@@ -307,7 +307,7 @@ struct PROCESS {
 	int sleepTime;      ///< Number of scheduler cycles to sleep.
 	uint32 pid;         ///< Process ID.
 	uint32 pidWaiting[CORO_MAX_PID_WAITING];    ///< Process ID(s) that the process is currently waiting on.
-	char param[CORO_PARAM_SIZE];    ///< Process-specific information.
+	alignas(max_align_t) char param[CORO_PARAM_SIZE];    ///< Process-specific information.
 };
 typedef PROCESS *PPROCESS;
 


Commit: a831accde7137e279fdeb17b95189f249debcfae
    https://github.com/scummvm/scummvm/commit/a831accde7137e279fdeb17b95189f249debcfae
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2023-07-14T11:10:28+02:00

Commit Message:
TONY: Fix various types discrepancies

Avoid using byte * for opaque types and don't do unneeded casts.

Changed paths:
    engines/tony/mpal/expr.cpp
    engines/tony/mpal/expr.h
    engines/tony/mpal/memory.cpp
    engines/tony/mpal/memory.h


diff --git a/engines/tony/mpal/expr.cpp b/engines/tony/mpal/expr.cpp
index c1262df7156..be22e7c312c 100644
--- a/engines/tony/mpal/expr.cpp
+++ b/engines/tony/mpal/expr.cpp
@@ -39,7 +39,7 @@ namespace MPAL {
  * @param h				Handle to the original expression
  * @retruns		Pointer to the cloned expression
  */
-static byte *duplicateExpression(MpalHandle h) {
+static void *duplicateExpression(MpalHandle h) {
 	byte *orig, *clone;
 
 	orig = (byte *)globalLock(h);
@@ -144,9 +144,9 @@ static void solve(LpExpression one, int num) {
  * @param expr				Pointer to an expression duplicated by DuplicateExpression
  * @returns		Value
  */
-static int evaluateAndFreeExpression(byte *expr) {
-	int num = *expr;
-	LpExpression one = (LpExpression)(expr + 1);
+static int evaluateAndFreeExpression(void *expr) {
+	int num = *(byte *)expr;
+	LpExpression one = (LpExpression)((byte *)expr + 1);
 
 	// 1) Substitutions of variables
 	LpExpression cur = one;
@@ -185,7 +185,7 @@ static int evaluateAndFreeExpression(byte *expr) {
 const byte *parseExpression(const byte *lpBuf, const Common::UnalignedPtr<MpalHandle> &h) {
 	byte *start;
 
-	uint32 num = *lpBuf;
+	byte num = *lpBuf;
 	lpBuf++;
 
 	if (num == 0)
@@ -196,7 +196,7 @@ const byte *parseExpression(const byte *lpBuf, const Common::UnalignedPtr<MpalHa
 		return NULL;
 
 	start = (byte *)globalLock(h.load());
-	*start = (byte)num;
+	*start = num;
 
 	LpExpression cur = (LpExpression)(start + 1);
 
@@ -270,8 +270,8 @@ bool compareExpressions(MpalHandle h1, MpalHandle h2) {
 	e1 = (byte *)globalLock(h1);
 	e2 = (byte *)globalLock(h2);
 
-	int num1 = *(byte *)e1;
-	int num2 = *(byte *)e2;
+	int num1 = *e1;
+	int num2 = *e2;
 
 	if (num1 != num2) {
 		globalUnlock(h1);
diff --git a/engines/tony/mpal/expr.h b/engines/tony/mpal/expr.h
index 67cb4efb5d0..7dedec46d2d 100644
--- a/engines/tony/mpal/expr.h
+++ b/engines/tony/mpal/expr.h
@@ -74,7 +74,7 @@ typedef struct {
 		int _num;        // Identifier (if type == ELT_NUMBER)
 		char *_name;     // Variable name (if type == ELT_VAR)
 		MpalHandle _son; // Handle expressions (if type == ELT_PARENTH)
-		byte *_pson;     // Handle lockato (if type == ELT_PARENTH2)
+		void *_pson;     // Handle lockato (if type == ELT_PARENTH2)
 	} _val;
 
 	byte _symbol;        // Mathematic symbols (see #define OP_*)
diff --git a/engines/tony/mpal/memory.cpp b/engines/tony/mpal/memory.cpp
index 1415c70aecc..2a1a1274052 100644
--- a/engines/tony/mpal/memory.cpp
+++ b/engines/tony/mpal/memory.cpp
@@ -102,7 +102,7 @@ void MemoryManager::destroyItem(MpalHandle handle) {
 /**
  * Locks an item for access
  */
-byte *MemoryManager::lockItem(MpalHandle handle) {
+void *MemoryManager::lockItem(MpalHandle handle) {
 	MemoryItem *item = (MemoryItem *)handle;
 	assert(item->_id == BLOCK_ID);
 	++item->_lockCount;
diff --git a/engines/tony/mpal/memory.h b/engines/tony/mpal/memory.h
index 4ada3b73dd5..e49e9275917 100644
--- a/engines/tony/mpal/memory.h
+++ b/engines/tony/mpal/memory.h
@@ -50,7 +50,7 @@ public:
 	static void freeBlock(MpalHandle handle);
 	static void destroyItem(MpalHandle handle);
 	static uint32 getSize(MpalHandle handle);
-	static byte *lockItem(MpalHandle handle);
+	static void *lockItem(MpalHandle handle);
 	static void unlockItem(MpalHandle handle);
 };
 


Commit: e7ee27e99ac0e85b5c27b4d120b74ed167e8f1f4
    https://github.com/scummvm/scummvm/commit/e7ee27e99ac0e85b5c27b4d120b74ed167e8f1f4
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2023-07-14T11:10:28+02:00

Commit Message:
TONY: Fix MemoryManager data alignment

Data being a byte type, it was never aligned.
Don't overallocate 1 byte which is here only because C++ doesn't support
VLA.

Changed paths:
    engines/tony/mpal/memory.cpp
    engines/tony/mpal/memory.h


diff --git a/engines/tony/mpal/memory.cpp b/engines/tony/mpal/memory.cpp
index 2a1a1274052..341e3de515c 100644
--- a/engines/tony/mpal/memory.cpp
+++ b/engines/tony/mpal/memory.cpp
@@ -36,7 +36,7 @@ namespace MPAL {
  * @return					Returns a MemoryItem instance for the new block
  */
 MpalHandle MemoryManager::allocate(uint32 size, uint flags) {
-	MemoryItem *newItem = (MemoryItem *)malloc(sizeof(MemoryItem) + size);
+	MemoryItem *newItem = (MemoryItem *)malloc(sizeof(MemoryItem) - sizeof(byte[1]) + size);
 	newItem->_id = BLOCK_ID;
 	newItem->_size = size;
 	newItem->_lockCount = 0;
diff --git a/engines/tony/mpal/memory.h b/engines/tony/mpal/memory.h
index e49e9275917..ca021637df2 100644
--- a/engines/tony/mpal/memory.h
+++ b/engines/tony/mpal/memory.h
@@ -35,7 +35,7 @@ struct MemoryItem {
 	uint32 _id;
 	uint32 _size;
 	int _lockCount;
-	byte _data[1];
+	alignas(max_align_t) byte _data[1];
 
 	// Casting for access to data
 	operator void *() { return &_data[0]; }


Commit: 1f1988f5ed545114da24431e1ce92a5fd90e841a
    https://github.com/scummvm/scummvm/commit/1f1988f5ed545114da24431e1ce92a5fd90e841a
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2023-07-14T11:10:28+02:00

Commit Message:
TONY: Properly align Expressions

Make sure Expression array is properly aligned inside the buffer
provided by GlobalAlloc.

Changed paths:
    engines/tony/mpal/expr.cpp


diff --git a/engines/tony/mpal/expr.cpp b/engines/tony/mpal/expr.cpp
index be22e7c312c..029e44d5d59 100644
--- a/engines/tony/mpal/expr.cpp
+++ b/engines/tony/mpal/expr.cpp
@@ -33,6 +33,8 @@ namespace Tony {
 
 namespace MPAL {
 
+static const size_t EXPR_LENGTH_SIZE = alignof(max_align_t);
+
 /**
  * Duplicate a mathematical expression.
  *
@@ -44,13 +46,13 @@ static void *duplicateExpression(MpalHandle h) {
 
 	orig = (byte *)globalLock(h);
 
-	int num = *(byte *)orig;
-	LpExpression one = (LpExpression)(orig+1);
+	int num = *orig;
+	LpExpression one = (LpExpression)(orig + EXPR_LENGTH_SIZE);
 
-	clone = (byte *)globalAlloc(GMEM_FIXED, sizeof(Expression) * num + 1);
-	LpExpression two = (LpExpression)(clone + 1);
+	clone = (byte *)globalAlloc(GMEM_FIXED, sizeof(Expression) * num + EXPR_LENGTH_SIZE);
+	LpExpression two = (LpExpression)(clone + EXPR_LENGTH_SIZE);
 
-	memcpy(clone, orig, sizeof(Expression) * num + 1);
+	memcpy(clone, orig, sizeof(Expression) * num + EXPR_LENGTH_SIZE);
 
 	for (int i = 0; i < num; i++) {
 		if (one->_type == ELT_PARENTH) {
@@ -146,7 +148,7 @@ static void solve(LpExpression one, int num) {
  */
 static int evaluateAndFreeExpression(void *expr) {
 	int num = *(byte *)expr;
-	LpExpression one = (LpExpression)((byte *)expr + 1);
+	LpExpression one = (LpExpression)((byte *)expr + EXPR_LENGTH_SIZE);
 
 	// 1) Substitutions of variables
 	LpExpression cur = one;
@@ -191,14 +193,14 @@ const byte *parseExpression(const byte *lpBuf, const Common::UnalignedPtr<MpalHa
 	if (num == 0)
 		return NULL;
 
-	h.store(globalAllocate(GMEM_MOVEABLE | GMEM_ZEROINIT, num * sizeof(Expression) + 1));
+	h.store(globalAllocate(GMEM_MOVEABLE | GMEM_ZEROINIT, num * sizeof(Expression) + EXPR_LENGTH_SIZE));
 	if (h.load() == NULL)
 		return NULL;
 
 	start = (byte *)globalLock(h.load());
 	*start = num;
 
-	LpExpression cur = (LpExpression)(start + 1);
+	LpExpression cur = (LpExpression)(start + EXPR_LENGTH_SIZE);
 
 	for (uint32 i = 0;i < num; i++) {
 		cur->_type = *(lpBuf);
@@ -213,6 +215,7 @@ const byte *parseExpression(const byte *lpBuf, const Common::UnalignedPtr<MpalHa
 			break;
 
 		case ELT_VAR:
+			// As name is a byte, there is no alignment rule
 			cur->_val._name = (char *)globalAlloc(GMEM_FIXED | GMEM_ZEROINIT, (*lpBuf) + 1);
 			if (cur->_val._name == NULL)
 				return NULL;
@@ -279,8 +282,8 @@ bool compareExpressions(MpalHandle h1, MpalHandle h2) {
 		return false;
 	}
 
-	LpExpression one = (LpExpression)(e1 + 1);
-	LpExpression two = (LpExpression)(e2 + 1);
+	LpExpression one = (LpExpression)(e1 + EXPR_LENGTH_SIZE);
+	LpExpression two = (LpExpression)(e2 + EXPR_LENGTH_SIZE);
 
 	for (int i = 0; i < num1; i++) {
 		if (one->_type != two->_type || (i != num1 - 1 && one->_symbol != two->_symbol)) {
@@ -336,7 +339,7 @@ bool compareExpressions(MpalHandle h1, MpalHandle h2) {
 void freeExpression(MpalHandle h) {
 	byte *data = (byte *)globalLock(h);
 	int num = *data;
-	LpExpression cur = (LpExpression)(data + 1);
+	LpExpression cur = (LpExpression)(data + EXPR_LENGTH_SIZE);
 
 	for (int i = 0; i < num; ++i, ++cur) {
 		switch (cur->_type) {


Commit: 76fd10dafe356053d130166b7db58aca20374820
    https://github.com/scummvm/scummvm/commit/76fd10dafe356053d130166b7db58aca20374820
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2023-07-14T11:10:28+02:00

Commit Message:
TONY: Fix _num type discrepancy

It's int32 everywhere outside expr anyway.

Changed paths:
    engines/tony/mpal/expr.cpp
    engines/tony/mpal/expr.h


diff --git a/engines/tony/mpal/expr.cpp b/engines/tony/mpal/expr.cpp
index 029e44d5d59..68a1458ebda 100644
--- a/engines/tony/mpal/expr.cpp
+++ b/engines/tony/mpal/expr.cpp
@@ -68,7 +68,7 @@ static void *duplicateExpression(MpalHandle h) {
 	return clone;
 }
 
-static int Compute(int a, int b, byte symbol) {
+static int32 Compute(int32 a, int32 b, byte symbol) {
 	switch (symbol) {
 	case OP_MUL:
 		return a * b;
@@ -146,7 +146,7 @@ static void solve(LpExpression one, int num) {
  * @param expr				Pointer to an expression duplicated by DuplicateExpression
  * @returns		Value
  */
-static int evaluateAndFreeExpression(void *expr) {
+static int32 evaluateAndFreeExpression(void *expr) {
 	int num = *(byte *)expr;
 	LpExpression one = (LpExpression)((byte *)expr + EXPR_LENGTH_SIZE);
 
@@ -170,7 +170,7 @@ static int evaluateAndFreeExpression(void *expr) {
 
 	// 3) algebraic resolution
 	solve(one, num);
-	int val = one->_val._num;
+	int32 val = one->_val._num;
 	globalDestroy(expr);
 
 	return val;
@@ -253,9 +253,9 @@ const byte *parseExpression(const byte *lpBuf, const Common::UnalignedPtr<MpalHa
  * @param h					Handle to the expression
  * @returns		Numeric value
  */
-int evaluateExpression(MpalHandle h) {
+int32 evaluateExpression(MpalHandle h) {
 	lockVar();
-	int ret = evaluateAndFreeExpression(duplicateExpression(h));
+	int32 ret = evaluateAndFreeExpression(duplicateExpression(h));
 	unlockVar();
 
 	return ret;
diff --git a/engines/tony/mpal/expr.h b/engines/tony/mpal/expr.h
index 7dedec46d2d..1dee1110f6c 100644
--- a/engines/tony/mpal/expr.h
+++ b/engines/tony/mpal/expr.h
@@ -71,7 +71,7 @@ typedef struct {
 	byte _type;          // Object Type (see enum ExprListTypes)
 
 	union {
-		int _num;        // Identifier (if type == ELT_NUMBER)
+		int32 _num;      // Identifier (if type == ELT_NUMBER)
 		char *_name;     // Variable name (if type == ELT_VAR)
 		MpalHandle _son; // Handle expressions (if type == ELT_PARENTH)
 		void *_pson;     // Handle lockato (if type == ELT_PARENTH2)
@@ -114,7 +114,7 @@ const byte *parseExpression(const byte *lpBuf, const Common::UnalignedPtr<MpalHa
  * @param h					Handle to the expression
  * @returns		Numeric value
  */
-int evaluateExpression(MpalHandle h);
+int32 evaluateExpression(MpalHandle h);
 
 /**
  * Compare two mathematical expressions together


Commit: 5964c36ab46d5ec0c94663cf8b78934d62109a86
    https://github.com/scummvm/scummvm/commit/5964c36ab46d5ec0c94663cf8b78934d62109a86
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2023-07-14T11:10:28+02:00

Commit Message:
COMMON: Don't use a pointer to T in UnalignedPtr

This lets the compiler think the pointer value could be aligned.

Changed paths:
    common/ptr.h


diff --git a/common/ptr.h b/common/ptr.h
index 472b41ca668..69155cf77a4 100644
--- a/common/ptr.h
+++ b/common/ptr.h
@@ -785,7 +785,7 @@ public:
 	void store(const T &value) const;
 
 private:
-	T *_ptr;
+	void *_ptr;
 };
 
 template<class T>




More information about the Scummvm-git-logs mailing list