Skip to content

Fixed compatibility check for versions connections #292

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JohnAmadis merged 5 commits into from
Mar 11, 2026
Merged

Fixed compatibility check for versions connections #292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
81323a1
Fixed compatibility check for versions connections
JohnAmadis Mar 11, 2026
a5f4ea2
Initial plan
Copilot Mar 11, 2026
ce6cbfa
Add ApiSignature unit tests and fix NULL dereference crashes
Copilot Mar 11, 2026
f7f398e
Add complex version string tests (dotted, multi-segment, multi-digit,…
Copilot Mar 11, 2026
9f3fcfa
Merge pull request #293 from choco-technologies/copilot/sub-pr-292
JohnAmadis Mar 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion inc/dmod.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ extern const char* Dmod_ApiSignature_GetModule( const char* Signature );
extern bool Dmod_ApiSignature_ReadModuleName( const char* Signature, char* ModuleName, size_t MaxLength );
extern bool Dmod_ApiSignature_ReadVersion( const char* Signature, char* Version, size_t MaxLength );
extern bool Dmod_ApiSignature_ReadModuleVersion( const char* Signature, char* ModuleVersion, size_t MaxLength );
extern bool Dmod_ApiSignature_AreEqual( const char* Signature1, const char* Signature2 );
extern bool Dmod_ApiSignature_AreCompatible( const char* Signature1, const char* Signature2 );
extern bool Dmod_ApiSignature_IsMal( const char* Signature );
extern bool Dmod_ApiSignature_IsBuiltin( const char* Signature );

Expand Down
94 changes: 82 additions & 12 deletions src/common/dmod_common.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ static const char* ApiSignature_GetModuleVersion( const char* Signature );
static const char* ApiSignature_GetModule( const char* Signature );
static bool ApiSignature_AreNamesEqual( const char* Signature1, const char* Signature2 );
static bool ApiSignature_AreModulesEqual( const char* Signature1, const char* Signature2 );
static bool ApiSignature_AreVersionsEqual( const char* Signature1, const char* Signature2 );
static bool ApiSignature_AreApiVersionsCompatible( const char* Signature1, const char* Signature2 );
static bool ApiSignature_AreModuleVersionsCompatible( const char* Signature1, const char* Signature2 );
static bool ApiSignature_AreVersionsCompatible( const char* Signature1, const char* Signature2 );

//==============================================================================
// FUNCTION IMPLEMENTATIONS
Expand Down Expand Up @@ -228,6 +230,10 @@ bool Dmod_ApiSignature_ReadModuleName( const char* Signature, char* ModuleName,
{
return false;
}
if( ModuleName == NULL )
{
return false;
}
if( !Dmod_ApiSignature_IsValid( Signature ) )
{
return false;
Expand Down Expand Up @@ -273,6 +279,10 @@ bool Dmod_ApiSignature_ReadVersion( const char* Signature, char* Version, size_t
{
return false;
}
if( Version == NULL )
{
return false;
}
if( !Dmod_ApiSignature_IsValid( Signature ) )
{
return false;
Expand Down Expand Up @@ -313,6 +323,10 @@ bool Dmod_ApiSignature_ReadModuleVersion( const char* Signature, char* ModuleVer
{
return false;
}
if( ModuleVersion == NULL )
{
return false;
}
if( !Dmod_ApiSignature_IsValid( Signature ) )
{
return false;
Expand All @@ -339,14 +353,14 @@ bool Dmod_ApiSignature_ReadModuleVersion( const char* Signature, char* ModuleVer
}

/**
* @brief Check if API signatures are equal
* @brief Check if API signatures are compatible
*
* @param Signature1 First signature
* @param Signature2 Second signature
*
* @return true if signatures are equal, false otherwise
* @return true if signatures are compatible, false otherwise
*/
bool Dmod_ApiSignature_AreEqual( const char* Signature1, const char* Signature2 )
bool Dmod_ApiSignature_AreCompatible( const char* Signature1, const char* Signature2 )
{
if( !Dmod_ApiSignature_IsValid( Signature1 ) || !Dmod_ApiSignature_IsValid( Signature2 ) )
{
Expand All @@ -355,7 +369,7 @@ bool Dmod_ApiSignature_AreEqual( const char* Signature1, const char* Signature2

Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dmod_ApiSignature_AreCompatible() only checks Dmod_ApiSignature_IsValid(), but IsValid() accepts signature types that don't include '@' / ':' (e.g. IRQ signatures). The helpers called here (ApiSignature_AreModulesEqual(), ApiSignature_AreApiVersionsCompatible()) assume those delimiters exist and can dereference NULL from strchr(), leading to a crash on valid inputs. Consider either tightening IsValid() for the signatures this function supports, or adding delimiter/prefix checks here and returning false (or falling back to strict string compare) when required fields are missing.

Suggested change
/*
* Some valid signature types (e.g. IRQ signatures) do not contain the
* '@' / ':' delimiters expected by the internal ApiSignature_* helpers.
* Only call those helpers when both signatures have the required
* delimiters; otherwise, fall back to strict string comparison to avoid
* dereferencing NULL pointers from strchr().
*/
const char* at1 = strchr( Signature1, '@' );
const char* colon1 = strchr( Signature1, ':' );
const char* at2 = strchr( Signature2, '@' );
const char* colon2 = strchr( Signature2, ':' );
bool hasStructured1 = ( at1 != NULL ) && ( colon1 != NULL ) && ( at1 < colon1 );
bool hasStructured2 = ( at2 != NULL ) && ( colon2 != NULL ) && ( at2 < colon2 );
if( !hasStructured1 || !hasStructured2 )
{
/* For non-structured signatures, treat compatibility as exact match. */
return strcmp( Signature1, Signature2 ) == 0;
}

Copilot uses AI. Check for mistakes.
return ApiSignature_AreNamesEqual( Signature1, Signature2 )
&& ApiSignature_AreModulesEqual( Signature1, Signature2 )
&& ApiSignature_AreVersionsEqual( Signature1, Signature2 );
&& ApiSignature_AreVersionsCompatible( Signature1, Signature2 );
}

/**
Expand Down Expand Up @@ -472,7 +486,7 @@ static bool ApiSignature_AreNamesEqual( const char* Signature1, const char* Sign
const char* name1 = ApiSignature_GetName( Signature1 );
const char* name2 = ApiSignature_GetName( Signature2 );

while( *name1 != '\0' && *name2 != '\0' && *name1 != ':' && *name2 != ':' )
while( *name1 != '\0' && *name2 != '\0' && *name1 != '@' && *name2 != '@' )
{
if( *name1 != *name2 )
{
Expand All @@ -498,7 +512,12 @@ static bool ApiSignature_AreModulesEqual( const char* Signature1, const char* Si
const char* module1 = ApiSignature_GetModule( Signature1 );
const char* module2 = ApiSignature_GetModule( Signature2 );

while( *module1 != '\0' && *module2 != '\0' && *module1 != '@' && *module2 != '@' )
if( module1 == NULL || module2 == NULL )
{
return module1 == module2;
}

while( *module1 != '\0' && *module2 != '\0' && *module1 != ':' && *module2 != ':' )
{
if( *module1 != *module2 )
{
Expand All @@ -512,23 +531,28 @@ static bool ApiSignature_AreModulesEqual( const char* Signature1, const char* Si
}

/**
* @brief Check if versions are equal
* @brief Check if API versions are compatible
*
* @param Signature1 First signature
* @param Signature2 Second signature
*
* @return true if versions are equal, false otherwise
* @return true if API versions are compatible, false otherwise
*/
static bool ApiSignature_AreVersionsEqual( const char* Signature1, const char* Signature2 )
static bool ApiSignature_AreApiVersionsCompatible( const char* Signature1, const char* Signature2 )
{
const char* version1 = ApiSignature_GetVersion( Signature1 );
const char* version2 = ApiSignature_GetVersion( Signature2 );

while( *version1 != '\0' && *version2 != '\0' && *version1 != '.' && *version2 != '.' )
if( version1 == NULL || version2 == NULL )
{
return version1 == version2;
}

while( *version1 != '\0' && *version2 != '\0' && *version1 != '/' && *version2 != '/' && *version1 != '.' && *version2 != '.' )
{
if( *version1 != *version2 )
{
DMOD_LOG_ERROR("Version mismatch: %s != %s\n", version1, version2);
DMOD_LOG_ERROR("API version mismatch: %s != %s\n", version1, version2);
return false;
}
version1++;
Expand All @@ -537,3 +561,49 @@ static bool ApiSignature_AreVersionsEqual( const char* Signature1, const char* S

return *version1 == *version2;
}

/**
* @brief Check if module versions are equal
*
* @param Signature1 First signature
* @param Signature2 Second signature
*
* @return true if module versions are compatible, false otherwise
*/
Comment on lines +565 to +572
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docstring inconsistency: the @brief says "Check if module versions are equal" but the function name/return description are about compatibility. Update the brief to match the compatibility semantics to avoid misleading generated docs.

Copilot uses AI. Check for mistakes.
static bool ApiSignature_AreModuleVersionsCompatible( const char* Signature1, const char* Signature2 )
{
const char* moduleVersion1 = ApiSignature_GetModuleVersion( Signature1 );
const char* moduleVersion2 = ApiSignature_GetModuleVersion( Signature2 );

if( moduleVersion1 == NULL || moduleVersion2 == NULL )
{
return moduleVersion1 == moduleVersion2;
}

while( *moduleVersion1 != '\0' && *moduleVersion2 != '\0' && *moduleVersion1 != '.' && *moduleVersion2 != '.' )
{
if( *moduleVersion1 != *moduleVersion2 )
{
DMOD_LOG_ERROR("Module version mismatch: %s != %s\n", moduleVersion1, moduleVersion2);
return false;
}
moduleVersion1++;
moduleVersion2++;
}

return *moduleVersion1 == *moduleVersion2;
}
Comment on lines +573 to +595
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ApiSignature_AreModuleVersionsCompatible() dereferences moduleVersion1/moduleVersion2 without checking for NULL. ApiSignature_GetModuleVersion() returns NULL when the signature version has no '/' (e.g. signatures like "...:1.0" exist in the repo), which will crash when Dmod_ApiSignature_AreCompatible() is used (e.g. from Dmod_GetFunction). Handle missing module-version gracefully (e.g., treat "no module version" as compatible when both are missing, or skip module-version comparison when either side lacks it) before entering the loop.

Copilot uses AI. Check for mistakes.

/**
* @brief Check if versions are compatible
*
* @param Signature1 First signature
* @param Signature2 Second signature
*
* @return true if versions are compatible, false otherwise
*/
static bool ApiSignature_AreVersionsCompatible( const char* Signature1, const char* Signature2 )
{
return ApiSignature_AreApiVersionsCompatible( Signature1, Signature2 )
&& ApiSignature_AreModuleVersionsCompatible( Signature1, Signature2 );
}
2 changes: 1 addition & 1 deletion src/system/dmod_system.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2051,7 +2051,7 @@ void* Dmod_GetDifFunction( Dmod_Context_t* Context, const char* DifSignature )
size_t numberOfEntries = Dmod_Api_GetNumberOfEntries( &Context->Inputs );
for(size_t i = 0; i < numberOfEntries; i++)
{
if( Dmod_ApiSignature_AreEqual( Context->Inputs.InputSection->Entries[i].Signature, DifSignature ) )
if( Dmod_ApiSignature_AreCompatible( Context->Inputs.InputSection->Entries[i].Signature, DifSignature ) )
{
return Context->Inputs.InputSection->Entries[i].Function;
}
Expand Down
6 changes: 3 additions & 3 deletions src/system/public/dmod_dmf_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ bool Dmod_ConnectApi( Dmod_Api_t* OutputsApi, Dmod_Api_t* InputsApi )
{
continue;
}
else if( Dmod_ApiSignature_AreEqual( OutputsApi->OutputSection->Entries[i], InputsApi->InputSection->Entries[j].Signature ) )
else if( Dmod_ApiSignature_AreCompatible( OutputsApi->OutputSection->Entries[i], InputsApi->InputSection->Entries[j].Signature ) )
{
DMOD_LOG_VERBOSE("Connected: %s 0x%08X\n", InputsApi->InputSection->Entries[j].Signature, InputsApi->InputSection->Entries[j].Function);
OutputsApi->OutputSection->Entries[i] = InputsApi->InputSection->Entries[j].Function;
Expand Down Expand Up @@ -585,7 +585,7 @@ void* Dmod_GetFunction( Dmod_Context_t* Context, const char* Signature )
size_t numberOfEntries = Dmod_Api_GetNumberOfEntries( &Context->Inputs );
for(size_t i = 0; i < numberOfEntries; i++)
{
if( Dmod_ApiSignature_AreEqual( Context->Inputs.InputSection->Entries[i].Signature, Signature ) )
if( Dmod_ApiSignature_AreCompatible( Context->Inputs.InputSection->Entries[i].Signature, Signature ) )
{
return Context->Inputs.InputSection->Entries[i].Function;
}
Expand Down Expand Up @@ -652,7 +652,7 @@ Dmod_Context_t* Dmod_GetNextDifModule( const char* DifSignature, Dmod_Context_t*
size_t numberOfInputs = Dmod_Api_GetNumberOfEntries( &Dmod_Contexts[i]->Inputs );
for(size_t j = 0; j < numberOfInputs; j++)
{
if( Dmod_ApiSignature_AreEqual( Dmod_Contexts[i]->Inputs.InputSection->Entries[j].Signature, DifSignature ) )
if( Dmod_ApiSignature_AreCompatible( Dmod_Contexts[i]->Inputs.InputSection->Entries[j].Signature, DifSignature ) )
{
Dmod_ExitCritical();
return Dmod_Contexts[i];
Expand Down
1 change: 1 addition & 0 deletions tests/system/public/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@ set(TEST_SOURCES
${CMAKE_CURRENT_SOURCE_DIR}/tests_dmod_loadfile_package_path.cpp
${CMAKE_CURRENT_SOURCE_DIR}/tests_dmod_irq.cpp
${CMAKE_CURRENT_SOURCE_DIR}/tests_dmod_verify_apis.cpp
${CMAKE_CURRENT_SOURCE_DIR}/tests_dmod_api_signature.cpp
PARENT_SCOPE
)
Loading
Loading