Structures using pointers get wrong

Question

I made code of bob the builder, it's a structure, every bob the builder has name and two more integers (doesn't really matter). there are three functions

initialize the struct(with "bob" and 0 and 3)
the second function get two structures and it's needs to copy between those structures
the third function is to free the names(char*) of every bob.

Firstly, the second function(copy) went wrong in the debugging because it didn't copy the name(Need your help in analyzing why it happened) and secondly, the code crashed in the free function. Can someone tell me how to free names(char*) of structures?

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#define LENGTH_OF_BOB 4

typedef struct bobTheBuilder
{
    char* name;
    int fixed;
    int maxFix;
}bob;

//typedef struct bobTHeBuilder bob;

void deleteBob(bob currBob);
void initBob(bob *currBob);
void copyStruct(bob* dst, bob src);
int main(void)
{
    bob currBob = {0,0,0};
    bob secondBob;
    initBob(&currBob);
    copyStruct(&secondBob, currBob);
    deleteBob(currBob);
    deleteBob(secondBob);
    system("PAUSE");    
    return 0;
}
/*
*/
void initBob(bob *currBob)
{
    char* str = (char*)calloc(LENGTH_OF_BOB, sizeof(char));
    char string[] = "bob";
    if (str)
    {
        strcat(string, "\0");
        str = string;

        currBob->name = str;
        currBob->fixed = 0;
        currBob->maxFix = 3;
    }
}
/*
*/
void deleteBob(bob currBob)
{
    free(currBob.name);
}
void copyStruct(bob* dest, bob src)
{
    dest->fixed = src.fixed;
    dest->maxFix = src.maxFix;
    dest->name = (char*)malloc(sizeof(char) *LENGTH_OF_BOB);
    strncpy(dest->name, src.name, LENGTH_OF_BOB);
}

[Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). — Sourav Ghosh, Apr 06 '16 at 12:09
just when you think it can't get any worse, then `strncpy` kicks you in the gut — M.M, Apr 06 '16 at 12:57

kaylum · Accepted Answer · 2016-04-06T12:19:54.053

In initBob you have:

char* str = (char*)calloc(LENGTH_OF_BOB, sizeof(char));
char string[] = "bob";
str = string;
currBob->name = str;

That sets currBob->name to point to a local automatic variable. Not to the dynamcically allocated buffer. The automatic variable goes out of scope when the function exits and hence is no longer valid. And of course it cannot be freed as it is not dynamically allocated memory.

I'm not really sure what you are trying to do there. Besides incorrectly setting str to point to a local variable you also have an unnecessary strcat. I'm guessing you are trying to NUL terminate the buffer. But that is unnecessary as initialising an unsized char array with a string literal already guarantees NUL termination.

With these issues in mind, the initBob function should be more like:

void initBob(bob *currBob)
{
    currBob->name = calloc(LENGTH_OF_BOB, sizeof(char));
    if (currBob->name)
    {
        strcpy(currBob->name, "bob");  
        currBob->fixed = 0;
        currBob->maxFix = 3;
    }
}

score 0 · Answer 2 · answered Apr 06 '16 at 12:35

I do not know if it is just a simple example to learn how do it or if it is really your scope, but if you need to do this use :strdup()

void initBob(bob *currBob)
{ 
if (currBob->name)
{
    currBob->name=strdup("bob");  
    currBob->fixed = 0;
    currBob->maxFix = 3;
}
}

than you have to free() it somewhere, because malloc() the string... it is ANSI standard

Structures using pointers get wrong

2 Answers2