Skip to content
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.

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

fix: assert failures on null/empty maps (Closes #504) #509

Conversation

eaton-coreymutter
Copy link
Contributor

Sometimes objects intended to be maps are null, or initialized but type IOT_DATA_NULL, causing map get/iterate functions to assert. Bypass these cases, treating it as an empty map.

If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/device-sdk-c/blob/main/.github/Contributing.md

PR Checklist

Please check if your PR fulfills the following requirements:

  • [ x ] I am not introducing a breaking change (if you are, flag in conventional commit message with BREAKING CHANGE: describing the break)
  • [ x ] I am not introducing a new dependency (add notes below if you are)
  • I have added unit tests for the new feature or bug fix (no existing tests in this repo)
  • I have fully tested (add details below) this the new feature or bug fix (if not, why?) (based on a different SDK version than I use, no way to test other than it builds with no warnings)
  • I have opened a PR for the related docs change (if not, why?) (N/A)

Testing Instructions

New Dependency Instructions (If applicable)

Sometimes objects intended to be maps are null, or initialized but
type IOT_DATA_NULL, causing map get/iterate functions to assert.
Bypass these cases, treating it as an empty map.

Signed-off-by: Corey Mutter <[email protected]>
Copy link
Member

@SteveOss SteveOss left a comment

Choose a reason for hiding this comment

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

Not sure what version of the iot library this is against. But current version allows a NULL passed to iot_data_type, returning IOT_DATA_INVALID, in which case a NULL check before calling the function is not required.

@iain-anderson
Copy link
Member

Should be 1.5
iot_data_string_map_get_map does the type check for you, so you either get a map or NULL.

@iain-anderson
Copy link
Member

Also some of these, eg the overall config, are created as string maps within the SDK so we don't really need to check them. Just the external objects eg the implementation config defaults and data we receive via REST

Copy link
Member

@SteveOss SteveOss left a comment

Choose a reason for hiding this comment

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

LGTM

@SteveOss
Copy link
Member

Can't see anything wrong with code changes. @iain-anderson might be worth specifically identifying where we can be sure of data type, to eliminate any unnecessary type checking, although is harmless.

@iain-anderson
Copy link
Member

The main one would be that if we validate the driver configuration defaults driverdfls on entry to devsdk_service_start then we don't need to do further checks when either the sdk or driver config is passed around.

@eaton-coreymutter eaton-coreymutter merged commit 442156b into edgexfoundry:main Mar 28, 2024
3 checks passed
@eaton-coreymutter eaton-coreymutter deleted the bugfix/issue-504-assertion-avoidance branch March 28, 2024 12:31
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.

3 participants