Skip to content

Conversation

@DLST316
Copy link

@DLST316 DLST316 commented Sep 25, 2025

Summary

This Pull Request modifies the IModRepositoryBridge::filesAvailable signal to pass the list of file info objects by value (QList<ModRepositoryFileInfo>) instead of by a list of raw pointers (QList<ModRepositoryFileInfo*>).

This is a prerequisite change required to fix a critical dangling pointer bug in the main ModOrganizer2 application.

Context: The Downstream Bug

In the main application's repository, a critical dangling pointer bug was identified in NexusBridge::nxmFilesAvailable. This bug could lead to random application crashes. The full context and discussion can be found in the original Pull Request here:

ModOrganizer2/modorganizer#2288

During the discussion in that PR, it was determined that the root cause was the unsafe, pointer-based design of the filesAvailable signal in this IModRepositoryBridge interface.

Design Rationale: Modification vs. Addition

As discussed in the original PR, a key decision was whether to modify the existing signal or add a new, overloaded one for backward compatibility.

This PR modifies the existing signal directly. This is a deliberate breaking change made for the following reasons:

  1. Eliminating Unsafe Patterns: The original pointer-based signature is inherently dangerous and led to the bug. Leaving this "trap" in the API for future use would be a long-term risk to the stability of the codebase.
  2. Maintainer Guidance: The project maintainer explicitly suggested that "a proper fix would be to modify the interface to not pass pointers." This change directly implements that guidance.
  3. Improving API Safety: By enforcing a value-based signature, we guarantee that consumers of this API cannot accidentally misuse pointers, making the entire system more robust and safe.

BREAKING CHANGE

The signature of the IModRepositoryBridge::filesAvailable signal has been changed from const QList<ModRepositoryFileInfo*>& to const QList<ModRepositoryFileInfo>&.

All classes that implement this interface or slots that connect to this signal must be updated to handle a value-based QList.

This changes the signal to pass a list of objects by value instead of pointers. This is a prerequisite for fixing a dangling pointer bug in the main application and improves the overall API safety.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant