CODEWITHSUNDEEP
cfree
leafy_fingers
leafy_fingers

Reputation: 53

Using free() corrupts char array data in C

The problem:

I need to malloc a struct to populate a char *[64] array. This array gets corrupted when I free the struct. Specifically the first index. How should I deal with this? 

int main(void) {
char *names[64];
uint32_t aCount = 0;
uint32_t count = 0;
vkEnumerateInstanceExtensionProperties(NULL,&count, NULL);
VkExtensionProperties *extension_names = malloc(sizeof(VkExtensionProperties) * count);
vkEnumerateInstanceExtensionProperties(NULL,&count,extension_names);

for(uint32_t i = 0; i < count; i++) {
    names[aCount++] = extension_names[i].extensionName;
}

printf("First extension available: %s\n",names[0]);

free(extension_names);

printf("First extension available: %s\n",names[0]);
return 0;}

Here is the result:

Before free()

First extension available: VK_KHR_device_group_creation

After free()

First extension available: ���yUU

Upvotes: 1

Views: 139

Answers (3)

user3657941
user3657941

Reputation:

You can use strdup to make copies of the strings and solve the "use after free" problem you are having:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define UNUSED(x) \
    ((void)(x))

#define VK_MAX_EXTENSION_NAME_SIZE 256

typedef struct VkExtensionProperties {
    char        extensionName[VK_MAX_EXTENSION_NAME_SIZE];
    uint32_t    specVersion;
} VkExtensionProperties;

void vkEnumerateInstanceExtensionProperties(void *unused, 
        uint32_t *count, VkExtensionProperties *result) {
    UNUSED(unused);
    *count = 64;
    if (result) {
        for (int index = 0; index < *count; index++) {
            snprintf(result[index].extensionName, 
                    sizeof(result->extensionName), 
                    "extension%03d", 
                    index);
        }
    }
}

int main()
{
    char *names[64];
    uint32_t aCount = 0;
    uint32_t count = 0;
    vkEnumerateInstanceExtensionProperties(NULL, &count, NULL);
    VkExtensionProperties *extension_names = malloc(sizeof(VkExtensionProperties) * count);
    vkEnumerateInstanceExtensionProperties(NULL, &count, extension_names);

    for (uint32_t i = 0; i < count; i++) {
        names[aCount++] = strdup(extension_names[i].extensionName);
    }
    printf("First extension available: %s\n", names[0]);

    free(extension_names);

    printf("First extension available: %s\n", names[0]);

    return 0;
}

Output

First extension available: extension000
First extension available: extension000

I don't have Vulcan installed, so I simulated the behavior of the function you called.

Helpful GCC Flags

While I have your attention, don't forget to compile your code with -Wall -Werror to help you fix problems at compile time:

$ gcc -Wall -Werror -o program program.c

Upvotes: 2

Nikos C.
Nikos C.

Reputation: 51840

You freed the strings, so not sure what you expected to happen. The second printf() will access freed memory. If you want to keep the strings around longer, you should copy the string data, not just the pointers to that data. Also, you need to write safer code to avoid buffer overflows (you're not checking if you're writing past the end of names, for example.)

for (uint32_t i = 0; i < count && aCount < 64; i++, aCount++) {
    // +1 for the '\0' terminator
    const size_t len = strlen(extension_names[i].extensionName) + 1;

    names[aCount] = malloc(len);
    memcpy(names[aCount], extension_names[i].extensionName, len);
}

Note that you now also have to free() each element in names when you no longer need it.

Upvotes: 2

Rishikesh Raje
Rishikesh Raje

Reputation: 8614

You are assigning names[aCount++] = extension_names[i].extensionName;

i.e. you are copying the extension_names in the array of pointers that is names.

You can only free extension_names after you are done with using names

Upvotes: 2

Related Questions