eclipse/paho.mqtt.c

Need to initialize heap before calling MQTTProperties_add

HammerCheng opened this issue · 4 comments

Describe the bug
I defined a std::mapstd::string,std::string userProps, added 15 elements to this userProps, then looped through userProps and called MQTTProperties_add to add to MQTTProperties, and the test found that the size of userProps was less than or equal to 10. Execute No problem, the program crashes when it is greater than 10。

To Reproduce
code:
MQTTAsync_connectOptions m_connectOptions;
MQTTProperties properties = MQTTProperties_initializer;
for (const auto& prop : m_connectionParam.userProps)//m_connectionParam.userProps is map.
{
try
{
std::string u8Key = GBKToUTF8(prop.first.c_str());
std::string u8Value = GBKToUTF8(prop.second.c_str());

  MQTTProperty property;
  property.identifier = MQTTPROPERTY_CODE_USER_PROPERTY;
  property.value.data.data = (char*)u8Key.c_str();
  property.value.data.len = (int)u8Key.length();
  property.value.value.data = (char*)u8Value.c_str();
  property.value.value.len = (int)u8Value.length();
  MQTTProperties_add(&properties, &property);

}
catch (std::exception e)
{
LOG4_ERROR("add property exeception, prop key:", prop.first);
}
}

crash code(realloc):
nt MQTTProperties_add(MQTTProperties* props, const MQTTProperty* prop)
{
int rc = 0, type;

if ((type = MQTTProperty_getType(prop->identifier)) < 0)
{
/StackTrace_printStack(stdout);/
rc = MQTT_INVALID_PROPERTY_ID;
goto exit;
}
else if (props->array == NULL)
{
props->max_count = 10;
props->array = malloc(sizeof(MQTTProperty) * props->max_count);
}
else if (props->count == props->max_count)
{
props->max_count += 10;
props->array = realloc(props->array, sizeof(MQTTProperty) * props->max_count);
}
...

Expected behavior
Does not crash and can connect to the mqtt service normally.

Screenshots
no.

Log files
no.

** Environment (please complete the following information):**

  • OS: Win32
  • Version:Eclipse Paho Asynchronous MQTT C Client Library, Version:1.3.12

Additional context
Finally, I solved it through the following code, pre-allocating enough memory to properties.array.
code:
MQTTProperties properties = MQTTProperties_initializer;
properties.max_count = m_connectionParam.userProps.size();//m_connectionParam.userProps is map.
properties.array = (MQTTProperty*)malloc(sizeof(MQTTProperty) * properties.max_count);

I can add more than 10 properties and then print out the results - code attached.

test.zip

Maybe there's a difference in what we're doing?

int main()
{
MQTTProperty property;
MQTTProperties props = MQTTProperties_initializer;
int i = 0;
int properties_count = 25;

for (i = 0; i < properties_count; ++i)
{
	char name[50];
	char value[50];
	int len;

	property.identifier = MQTTPROPERTY_CODE_USER_PROPERTY;

	len = sprintf(name, "test user property %d", i);
	property.value.data.data = name;
	property.value.data.len = len;

	len = sprintf(value, "test user property value %d", i);
	property.value.value.data = value;
	property.value.value.len = len;

	MQTTProperties_add(&props, &property);
}

printf("Number of props %d\n", props.count);
logProperties(&props);

MQTTProperties_free(&props);

}

Thanks for the test code. When there are more than 10 properties, the props data will be cleared.The source code of the mqtt library is as follows:
//MQTTProperties_add
if (props->count == props->max_count)
{
props->max_count += 10;
props->array = realloc(props->array, sizeof(MQTTProperty) * props->max_count);
}
//realloc TreeRemoveKey return null
void myrealloc(char file, int line, void* p, size_t size)
{
void* rc = NULL;
storageElement* s = NULL;

Thread_lock_mutex(heap_mutex);
s = TreeRemoveKey(&heap, ((eyecatcherType*)p)-1);
if (s == NULL)
	Log(LOG_ERROR, 13, "Failed to reallocate heap item at file %s line %d", file, line);
else
{
	size_t space = sizeof(storageElement);
	size_t filenamelen = strlen(file)+1;

	checkEyecatchers(file, line, p, s->size);
	size = Heap_roundup(size);
	state.current_size += size - s->size;
	if (state.current_size > state.max_size)
		state.max_size = state.current_size;
	if ((s->ptr = realloc(s->ptr, size + 2*sizeof(eyecatcherType))) == NULL)
	{
		Log(LOG_ERROR, 13, errmsg);
		goto exit;
	}
	space += size + 2*sizeof(eyecatcherType) - s->size;
	*(eyecatcherType*)(s->ptr) = eyecatcher; /* start eyecatcher */
	*(eyecatcherType*)(((char*)(s->ptr)) + (sizeof(eyecatcherType) + size)) = eyecatcher; /* end eyecatcher */
	s->size = size;
	space -= strlen(s->file);
	s->file = realloc(s->file, filenamelen);
	space += filenamelen;
	strcpy(s->file, file);
	s->line = line;
	rc = s->ptr;
	TreeAdd(&heap, s, space);
}

exit:
Thread_unlock_mutex(heap_mutex);
return (rc == NULL) ? NULL : ((eyecatcherType*)(rc)) + 1; /* skip start eyecatcher */
}

fix:
Execute MQTTAsync_createWithOptions first, the heap management will be initialized.

Ah, because the memory system in not initialized. You can call MQTTAsync_global_init() too, or build with HIGH_PERFORMANCE.

Updated doc.