Various compiler warning fixes by dpogue · Pull Request #1800 · H-uru/Plasma

dpogue · 2025-09-24T17:18:23Z

Fixes various warnings (mostly in CoreLib) from Xcode and Visual Studio around uninitialized local variables, unused values, improper use of getcwd, and some (but not all) signed/unsigned and int sizing warnings in hsStream.

This also enables CMP0156 which is supposed to deduplicate linked libraries to avoid warnings from Xcode.

I've tried to keep each commit reasonably self-contained, so we can drop anything that we don't have agreement on.

Sources/Plasma/CoreLib/plFileSystem.cpp

Sources/Plasma/NucleusLib/pnEncryption/plChecksum.cpp

Sources/Plasma/CoreLib/hsSystemInfo.cpp

dgelessus

This generally looks good.

I'm a little cautious about size_t in the context of file IO, because when done wrong, it can introduce problems with large files that go unnoticed in 64-bit builds and only affect 32-bit builds.

This isn't a realistic problem for us right now, because so much of the hsStream code hardcodes file sizes/offsets as uint32_t (and fixing that isn't the topic of this PR). But to avoid future issues, we should use size_t in IO code only for sizes/offsets of memory buffers (as for Read/Write for example). Sizes/offsets within files/streams should not be size_t. For now, unfortunately, the appropriate type is uint32_t, but at some point we should add an equivalent of POSIX off_t for file sizes/offsets. I've commented on the specific cases where I think size_t isn't appropriate.

dgelessus · 2025-09-27T15:49:36Z

Sources/Plasma/CoreLib/plFileSystem.cpp

    }
 #else
-    char *cwd_a = getcwd(nullptr, 0);
+    char cwd_a[1024];


Suggested change

char cwd_a[1024];

char cwd_a[PATH_MAX];

The super-ideal POSIX solution would be to call pathconf to dynamically get PATH_MAX for the path ".", but we already use the PATH_MAX macro constant in a few other places, so for now this is fine I think. (Plus, according to gnulib, many getcwd implementations don't work properly for paths longer than PATH_MAX anyway.)

Sources/Plasma/CoreLib/plFileSystem.cpp

dgelessus · 2025-09-27T16:03:19Z

Sources/Plasma/NucleusLib/pnEncryption/plChecksum.h

-        // Note: GetAsHexString() returns a pointer to a static string;
-        // do not rely on the contents of this string between calls!


good riddance!

dgelessus · 2025-09-27T17:00:34Z

Sources/Plasma/PubUtilLib/plFile/plSecureStream.cpp

-    uint32_t startChunkPos = startPos % kEncryptChunkSize;
+    size_t startChunkPos = startPos % kEncryptChunkSize;
    // Amount of data in the partial first chunk (0 if we're aligned)
-    uint32_t startAmt = (startChunkPos != 0) ? std::min(kEncryptChunkSize - startChunkPos, bytes) : 0;
+    size_t startAmt = (startChunkPos != 0) ? std::min(kEncryptChunkSize - startChunkPos, bytes) : 0;

-    uint32_t totalNumRead = IRead(bytes, buffer);
+    size_t totalNumRead = IRead(bytes, buffer);

-    uint32_t numMidChunks = (totalNumRead - startAmt) / kEncryptChunkSize;
-    uint32_t endAmt = (totalNumRead - startAmt) % kEncryptChunkSize;
+    size_t numMidChunks = (totalNumRead - startAmt) / kEncryptChunkSize;
+    size_t endAmt = (totalNumRead - startAmt) % kEncryptChunkSize;


Same story as for plEncryptedStream - offsets should stay uint32_t for now.

Sources/Tools/plPythonPack/main.cpp

dgelessus · 2025-09-27T17:13:11Z

Sources/Plasma/NucleusLib/pnSceneObject/plSceneObject.cpp

    for (size_t i = nOldMods; i < nOldMods + nNewMods; i++)
    {
-        refMsg = new plObjRefMsg(GetKey(), plRefMsg::kOnCreate, i, plObjRefMsg::kModifier);
+        refMsg = new plObjRefMsg(GetKey(), plRefMsg::kOnCreate, static_cast<int8_t>(i), plObjRefMsg::kModifier);


Feels like it would be better to allow larger indices in plObjRefMsg itself?

We can't change this without changing the stream layout for the message. There really shouldn't be any RefMsgs serialized anywhere, but it's not impossible, so we need to be careful about that.

I think that we have, in a few places, widened the internal storage and added something like hsAssert(fValue <=std::numeric_limits<oldType_t>::max(), "overflow"); to the serialization routine (example). It might not be a bad idea to do that.

plAttachMsg is a plRefMsg that can go over the wire, btw. It's pretty trivial to send those from Python. Thankfully, plAttachMsg is not a subclass of plObjRefMsg.

Sources/Plasma/PubUtilLib/plScene/plSceneNode.cpp

Co-Authored-By: Adam Johnson <AdamJohnso@gmail.com>

Behaviour is undefined if the buffer parameter is null. Co-Authored-By: dgelessus <dgelessus@users.noreply.github.com>

Co-Authored-By: dgelessus <dgelessus@users.noreply.github.com>

dgelessus · 2026-03-27T23:36:07Z

Looking at this again, would it make sense to split the integer size and signedness commits into a separate PR? All the other commits look like straightforward, independent fixes - they don't need to be blocked on figuring out the details of the integer types.

In my last review, there's only one remaining comment that's not about integer types, which is that the result array for getcwd should use PATH_MAX as the length, instead of hardcoding 1024.

Hoikas reviewed Sep 24, 2025

View reviewed changes

dpogue force-pushed the warning-fixes branch 5 times, most recently from 6fa8b1f to 84cd602 Compare September 27, 2025 05:54

dgelessus suggested changes Sep 27, 2025

View reviewed changes

Hoikas marked this pull request as draft January 31, 2026 18:56

dpogue force-pushed the warning-fixes branch 2 times, most recently from 8dbe5d0 to 2ad5dda Compare March 10, 2026 01:40

dpogue and others added 10 commits March 21, 2026 18:57

Fix warnings about duplicate linking on macOS

48c3f2f

Fix some warnings about uninitialized variables

fc291ac

Fix warning about Clang-specific behaviour

7bb612f

Co-Authored-By: Adam Johnson <AdamJohnso@gmail.com>

Fix improper use of getcwd function

2a3437a

Behaviour is undefined if the buffer parameter is null. Co-Authored-By: dgelessus <dgelessus@users.noreply.github.com>

Fix warnings in plFileSystem

c67f0a8

Co-Authored-By: dgelessus <dgelessus@users.noreply.github.com>

Replace sprintf with ST::string for checksums

cd7853a

Fix pfPython missing return from non-void method

9e6c9b5

Fix some integer size warnings in hsStream

d22fc5a

Co-Authored-By: dgelessus <dgelessus@users.noreply.github.com>

Fix some signed/unsigned comparison warnings

b865afc

Co-Authored-By: dgelessus <dgelessus@users.noreply.github.com>

Fix memset warning in plVertCoder

7a0c28c

dpogue force-pushed the warning-fixes branch from 0de3a3b to 7a0c28c Compare March 22, 2026 01:57

		// Note: GetAsHexString() returns a pointer to a static string;
		// do not rely on the contents of this string between calls!

Conversation

dpogue commented Sep 24, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dgelessus left a comment

Choose a reason for hiding this comment

Uh oh!

dgelessus Sep 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dgelessus Sep 27, 2025

Choose a reason for hiding this comment

Uh oh!

dgelessus Sep 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dgelessus Sep 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dpogue Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Hoikas Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dgelessus commented Mar 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dgelessus Sep 27, 2025 •

edited

Loading