C Programming - My program is good enough for my assignment but I know its not good

Question

Hi there

I'm just starting an assignment for uni and it's raised a question for me.

I don't understand how to return a string from a function without having a memory leak.

char* trim(char* line)  {
    int start = 0;
 int end = strlen(line) - 1;

 /* find the start position of the string */
 while(isspace(line[start]) != 0)  {
     start++;
 }
 //printf("start is %d\n", start);

 /* find the position end of the string */
 while(isspace(line[end]) != 0)  {
     end--;
 }
 //printf("end is %d\n", end);

 /* calculate string length and add 1 for the sentinel */
 int len = end - start + 2;

 /* initialise char array to len and read in characters */
 int i;
 char* trimmed = calloc(sizeof(char), len);

 for(i = 0; i < (len - 1); i++)  {
     trimmed[i] = line[start + i];
 }
 trimmed[len - 1] = '\0';

    return trimmed;
}

as you can see I am returning a pointer to char which is an array. I found that if I tried to make the 'trimmed' array by something like:

char trimmed[len];

then the compiler would throw up a message saying that a constant was expected on this line. I assume this meant that for some reason you can't use variables as the array length when initialising an array, although something tells me that can't be right.

So instead I made my array by allocating some memory to a char pointer.

I understand that this function is probably waaaaay sub-optimal for what it is trying to do, but what I really want to know is:

1. Can you normally initialise an array using a variable to declare the length like:

   char trimmed[len];

2. If I had an array that was of that type (char trimmed[]) would it have the same return type as a pointer to char (ie char*).

3. If I make my array by callocing some memory and allocating it to a char pointer, how do I free this memory. It seems to me that once I have returned this array, I can't access it to free it as it is a local variable.

Many thanks in advance

Joe

Answer 1

Aside from C++ solutions like using std::string, you could always allocate an array of a set size, and pass the size in as a parameter, and the array as a parameter by reference or as a pointer?

This way the data is allocated in the same scope and there is no memory leakage.

Then again you can always free the memory after calling. It means that the code responsible for creating it and the code responsible for destroying the data are not the same, which can be a precursor to mistakes and errors.

Answer 2

Allocation and deletion outside the function:

Allocate it from stack with char trimmed[SIZE] and pass it to the function or from heap with calloc and pass it to the function.

Allocation inside the function and deletion outside:

You allocate it inside the function with calloc, but the the caller must free it using free.

Answer 3

To address (3) - You can free the newly allocated string from your calling code, once you are finished with it:

char* tmp = trim(myline);

if (tmp != NULL) {
    ....
    free( tmp );
}

But this places a burden on the caller to remember to free the memory. So instead you might consider passing an allocated buffer and a buffer size to trim(), for example:

void trim(char* line, char *trimmed_buf, int trimmed_buf_len){ ... }

Neil did an excellent job addressing your other questions. Basically an array declaration char trimmed[len]; will declare a local variable on the stack, so while it is syntactically correct to return a char * to this memory, the memory location that it points to will no longer be valid.

Answer 4

To answer your specific questions, the syntax:

char trimmed[len];

where len is a variable, is only allowed in C99, not in C89 or in C++. The return type would indeed be a char *, but returning the local variable trimmed would result in undefined behaviour, so don't do it. And if you allocate an array dynamically in a function via calloc and return it, it is up to the caller of the function to free it, using the pointer that the function returns.

Answer 5

Hm, well, answers to your questions:

1. yes, the array would be allocated on the stack, not the heap and would be freed when the function returns.
2. yes, char[] is mostly equivalent to char*. It is best to keep those separate, as there is a semantic difference.
3. Using any pointer to the memory, you can use free. In your case, you would free the return the function. That is considered pretty bad form and bug-prone. You usually want the allocat'er to be the free()er. You could, perhaps, pass in a buffer for the new space, or the caller of the function could agree to have the char * line contents written on top of the existing contents. This works as trim() would always only remove stuff. I think the passed in buffer would work more often and is a good thing to get into the habit of.

As for your function, consider using memcpy() or its cousins to copy bytes into and out of a char buffer.

Answer 6

Regarding dynamic sizing an array declaration like char trimmed[len]; the newest version of the C standard (ISO/IEC 9899:1999) allows this, but for this function it would not help at all. The trimmed variable has its scope within the trim function and it is allocated on the stack. So if you put return trimmed; in your code you return a pointer to a variable on the stack, and the portion of the stack where this variable lives will be released at the point where the function returns, so that will not work out so well...

Answer 7

Have the caller pass in a pointer to a memory area (an a maximum length) to the function. That way the caller will be responsible for allocating (and deallocating) the memory, and the function will only provide output constrained to the buffer passed into the function.

Sure the person using the function could still mess things up (by specifying a length that doesn't have anything to do with actual buffer size, but then you can correctly argue it's the caller's fault, not this functions fault.