CODEWITHSUNDEEP
cmalloc
ant2009
ant2009

Reputation: 22486

using malloc for the life of the program

gcc 4.4.4 c89

I have always thought of using malloc for the life of the project for being the scope.

But I am just wondering if my idea is the best practice. For example, I initalize an instance of the struct in main. And create 2 functions for creating and destroying. I am just wondering if this is the right thing to do.

I have some skeleton code below.

Many thanks for any advice, 

typedef struct Network_dev_t {
    size_t id;
    char *name;
} Network_dev;

Network_dev* create_network_device(Network_dev *network)
{
    network = malloc(sizeof *network);
    if(network == NULL) {
        return NULL;
    }    
    return network;
}

void do_something(Network_dev *network)
{
    /* Do something with the network device */
}

void destroy_network_device(Network_dev *network)
{
    free(network);
}

int main(void)
{
    Network_dev *network = NULL;

    network = create_network_device(network);

    /* Do something with the network device */
    do_something(network);

    destroy_network_device(network);

    return 0;
}

Upvotes: 2

Views: 277

Answers (3)

Karmastan
Karmastan

Reputation: 5696

This code looks fine to me. I agree with pmg that your create_network_device could use a little work. Just to pull together what he said and make things clearer, here is exactly how I would write the function:

Network_dev *create_network_device()
{
    Network_dev *network = malloc(sizeof(*network));
    if (network) {
        network->id = 0;
        network->name = NULL;
    }
    return network;
}

Upvotes: 3

pmg
pmg

Reputation: 108978

Looks good.

I have a point or 2 about create_network_device

Network_dev* create_network_device(Network_dev *network)

no need to pass in a pointer; I'd rather have Network_dev* create_network_device(void)

{
    network = malloc(sizeof *network);

the if is not really necessary; if malloc failed the return network at the end of the function is the same as return NULL.

    if(network == NULL) {
        return NULL;
    }

If the allocation succeeded you might want to insure the struct members are in a know state here

    /* if (network) {       */
    /*     id = 0;          */
    /*     name = NULL;     */
    /* }                    */

    return network;
}

Upvotes: 4

Sjoerd
Sjoerd

Reputation: 75588

  • It is best to allocate memory and free memory in the same function. Just like you open and close files in the same function. You did this by creating and destroying a Network_dev in the main() function, which is good. This makes it easy to confirm that all malloced locations are also freed.
  • It is best to malloc() something as late as possible and free() it as soon as possible. That is, hold the memory for as short as possible. If your program's job is to do something with Network_dev, you did all right. If your program does a lot of other things, you should do them before malloc() or after free().

Upvotes: 2

Related Questions