allocating memory using calloc and freeing

Question

gcc 4.4.4 c89

I have a program that I am testing. I create a struct object called devname and allocate memory so that I can fill the elements. I display them and then free the memory that was allocated.

However, I am getting the following error:

invalid operands to binary != (have ‘struct Devices_names’ and ‘void *’)

That is in my for loop for displaying the structure elements. However, I feel I am testing for a NULL pointer.

Just a further question, is there an problem with the free?

Many thanks for any advice,

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

static struct Devices_names {
#define MAX_NAME_LEN 80
    int id;
    char name[MAX_NAME_LEN];
} *devname;

static void g_create_device_names(size_t devices);
static void g_get_device_names();
static void destroy_devices();

int main(void)
{
#define DEVICES 5
    g_create_device_names(DEVICES);

    g_get_device_names();

    destroy_devices();

    return 0;
}

static void g_create_device_names(size_t devices)
{
    size_t i = 0;
    devname = calloc(devices, sizeof *devname);
    if(devname == NULL) {
        exit(0);
    }

    for(i = 0; i < devices; i++) {
        devname[i].id = i;
        sprintf(devname[i].name, "device: %d", i);
    }
}

static void g_get_device_names()
{
    size_t i = 0;

    for(i = 0; devname[i] != NULL; i++) { <-- ERROR HERE
        printf("Device id --- [ %d ]\n", devname[i].id);
        printf("Device name - [ %s ]\n", devname[i].name);
    }
}

static void destroy_devices()
{
    while(devname != NULL) {
        free(devname++);
    }
}

Solution 1

Since you only have one allocation to create the whole devname array, you only need to check that array for NULL, and only need to free that one array. As you're looking through devname, each entry is actually a struct Devices_names, not a pointer, so it can't be compared with NULL or freed in any meaningful way. In this case, you will need a separate variable that tracks how many entries there are:

for (i = 0; i < devname_count; i++) {
    printf("Device id --- [ %d ]\n", devname[i].id);
    printf("Device name - [ %s ]\n", devname[i].name);
}

...

free(devname);
devname = NULL;
devname_count = 0;

Solution 2

devname[i] is not a pointer its a struct Devices_names, therefore the comparison doesn't make sense.

Solution 3

Where you write:

for(i = 0; devname[i] != NULL; i++) { <-- ERROR HERE

you are testing against NULL an instance of Device_names, not a pointer. It would be fine if you had an array of pointers to Device_names.

The other problem is that you are allocating only one Device_names, so you have not an array of them.

Solution 4

After calloc you need only test for returned pointer to be not-null (and calloc call succeeded).

But once you callocated an array you cannot determine how many items in allocation having only pointer to it, so neither devname[i] != NULL, nor devname+i != NULL will not work, altough second will compile. Only environment or RTL know this. And this is great difference between *alloc allocation and static declaration (even it is of variable size as introduced in C99). So you NEED to store size of allocated array elsewhere.

Also remember, array (or any other memory chunk) allocated with single calloc() should be deallocated with single free() call with SAME pointer as returned by malloc. Passing any other pointer to free() cause undefined behaviaour (which is often FAIL).

So your code should be:

static struct Devices_names {
#define MAX_NAME_LEN 80
    int id;
    char name[MAX_NAME_LEN];
} *devname;
size_t devicecount;

...

    devname = calloc(devices, sizeof *devname);
    if(devname == NULL) {
        exit(0);
    }
    devicecount = devices;

...

    for(i = 0; i<devicecount; i++) { // <-- no error more here

...

static void destroy_devices()
{
    free(devname);
}

View more solutions