1

What is the correct approach to dynamically allocate a struct array and its components in C? I have managed to do something that works,but I am kind of sceptical if it is correct. I have the following code:
This is my struct array that I need to dynamically allocate:

typedef struct
{
    char *wrong;
    char *right;
}Dictionary;

This is the function I call when I need to initialise my struct array:

Dictionary *init_Dictionary(int nr_elem)
{
    Dictionary *dict;
    dict = malloc(nr_elem*sizeof(Dictionary));
    for(int i=0; i<nr_elem; i++)
    {
        char wrong[101],right[101];
        scanf("%s%s",wrong,right);
        dict[i].wrong = malloc(strlen(wrong)*sizeof(char));
        dict[i].right = malloc(strlen(right)*sizeof(char));
        strcpy(dict[i].wrong,wrong);
        strcpy(dict[i].right,right);
    }
    return dict;
}

Then in my main function, I have this:

    int nr_elem;
    scanf("%d",&nr_elem);
    Dictionary *dict;
    dict = init_Dictionary(nr_elem);

Also,when I finish work with the struct, how do I free the used memory ?
EDIT Thank you all for the quick and indepth answers!

Improve this question
5
  • 2
    dict[i].wrong = malloc(strlen(wrong)*sizeof(char)); -> dict[i].wrong = malloc(strlen(wrong) + 1); (you need an extra char for the string terminator, and sizeof(char) is redundant). Commented Nov 27, 2018 at 8:56
  • 1
    Don't forget that char strings in C are really called null-terminated byte strings, and that the null-terminator needs space as well, and is not counted by strlen. Commented Nov 27, 2018 at 8:56
  • 1
    And you need to match each malloc (or similar) with a free. Commented Nov 27, 2018 at 8:57
  • @Someprogrammerdude so I have to loop through my struct array and call free on each component of the struct ? Commented Nov 27, 2018 at 9:00
  • 1
    Yes, and after the loop free the "array" of structures as well. Commented Nov 27, 2018 at 9:01

3 Answers 3

Reset to default
3

For each allocation you need to allocate one more location to allow for \0 (NULL terminator) at the end of the string. 

dict[i].wrong = malloc(strlen(wrong)*sizeof(char) +1 );
dict[i].right = malloc(strlen(right)*sizeof(char) +1);

To free, you first need to free all the pointers right and wrong in the array and then free the main dict array. Optionally, you can NULL the pointers after free.

Dictionary* freeDict(Dictionary *dict, int nr_elem)
{
    for (int i=0; i<nr_elem; i++)
    {
        free(dict->wrong);
        free(dict->right);
        dict->wrong = NULL;
        dict->right = NULL;
    }
    free (dict);
    dict = NULL;
    return dict;
}

//To call.
dict = free(dict, nr_elem);
Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

dict->wrong = NULL; That's pretty pointless, yeah? You are about to deallocate it anyway.
No it's not.It's always a good practice to set a pointer to null after free() to avoid reuse.
@TsakiroglouFotis Dogmatically following good practice rules without applying common sense is harmful. By reading this code, you can see that it is pointless. Or do you expect some maintainer of the code to crazy things with dict->wrong between free(dict->wrong); and free (dict);? That won't happen, so all it achieved was to slow down the freeDict function.
I understand what you say and I respect this.I just have another opinion.dict's scope here is above this function. dict created somewhere else and it might be out there in the code.So , let's assume that someone in the future takes your (let's say) 5.000 lines code and adds another free for dict.
2

The program design isn't good, you should separate UI from algorithms. Instead of this, you should first take the user input, then store it in 2 strings and pass the strings as parameters to init_Dictionary.

As for the allocation, it is almost correct. But you forgot to allocate space for the null terminator, it should be:

dict[i].wrong = malloc(strlen(wrong)+1);
dict[i].right = malloc(strlen(right)+1);

Multiplying with sizeof(char) isn't meaningful, since the definition of sizeof(char) is always 1 on all systems.

In a production-quality application, you must always check the result of each malloc, then handle errors.

You free memory the same way as you allocated it, but in the opposite order since you need dict itself to be valid until you have deallocated its members:

for(int i=0; i<nr_elem; i++)
{
  free(dict[i].wrong);
  free(dict[i].right);
}
free(dict);

As a rule of thumb, each call to malloc must be matched with a call to free.

Improve this answer

3 Comments

I would set all the pointers to null after freeing ,to avoid reuse.
@TsakiroglouFotis While that is certainly good practice, how feasible it is depends on the context. If you have a function such as void cleanup (Dictionary* d){ free(d); } then setting the pointer to NULL does no good, as it isn't returned to the caller.
I get your point.However when you are working with other people it's always good to avoid someone in the future break your code.For example someone takes your code and adds another free().
2

There's a bug in your implementation: strlen(s) does not count the terminating 0-character, so, despite one test may work successfully, this is actually an UB. strdup can do work for you; if you don't have it standard library, simply add 1 when allocating memory for string copies. Or even better: count string length once, then use this value to both allocate enough bytes and copy contents with memcpy.

Otherwise your algorithm is quite useful (provided an array of string pairs is really what you need, with no additional structure like search index or anything).

To deallocate it, add a destructor that performs element-wise deallocation and then frees the whole array:

void destroy(Dictionary *dict, size_t nr_elem) {
    for(size_t i = 0; i < nr_elem; ++i) {
        free(dict[i].wrong);
        free(dict[i].right);
    }
    free(dict);
}
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.