IOTechSystems / iotech-c-utils

Apache License 2.0
9 stars 7 forks source link

fix: assert failures on null/empty maps (edgexfoundry#504) #331

Closed eaton-coreymutter closed 5 months ago

eaton-coreymutter commented 6 months ago

Sometimes objects intended to be maps are null, or initialized but type IOT_DATA_NULL, causing map get/iterate functions to assert. Treat them as empty maps instead.

SteveOss commented 5 months ago

Essentially if you call a map function, you should both be passing a non NULL map. As the data type is generic the assertions help ensure data types are used correctly, making them tolerant to being passed the wrong type does not help. Also the master branch is not supported as a stable release, the branch tags are (EdgeX builds currently use packages off the v1.5-branch).

Returning duff values when passed a NULL can be wrong for example:

uint32_t iot_data_map_size (const iot_data_t map) { if ((!map) || (map->type != IOT_DATA_MAP)) { return 0; } return ((const iot_data_map_t) map)->size; }

Returning a map length of zero when passed a string is just wrong.

Some functions however can return something meaningful (and not incorrect) if passed a NULL:

iot_data_type_t iot_data_map_type (const iot_data_t * map) { if (!map) { return IOT_DATA_NULL; } return map->element_type; }

This is wrong as a map of NULL is a valid type, this cannot be determined if a NULL is passed. However something like:

iot_data_type_t iot_data_map_type (const iot_data_t * map) { assert ((map == NULL) || (map && map->type == IOT_DATA_MAP)) return map ? IOT_DATA_INVALID : map->element_type; }

Is better as the semantics in this context make sense. Note that if a non NULL argument is passed we still assert is a MAP.