Search code examples
cmemory-leakscharmallocfree

C - Memory Leak, function return Char*


Doing a refresh on C programming, and I'm having an issue with freeing memory. The below program gives me the below compiler warnings that I'm having a hard time solving for. Valgrind is also informing that there is a memory leak, but I am using free on the memory allocated with malloc(). Any guidance on what I am doing wrong when attempting to free memory on pointer 'alpha' in main() is appreciated?

Compiler Warning

In function ‘main’:/alphabetArrayPractice/src/main.c:37:10: warning: passing argument 1 of ‘free’ makes pointer from integer without a cast [-Wint-conversion]
   37 |     free(*alpha);
      |          ^~~~~~
      |          |
      |          char
In file included from /alphabetArrayPractice/src/main.c:2:
/usr/include/stdlib.h:565:25: note: expected ‘void *’ but argument is of type ‘char’
  565 | extern void free (void *__ptr) __THROW;

Valgrind Report

==950908== 
==950908== HEAP SUMMARY:
==950908==     in use at exit: 27 bytes in 1 blocks
==950908==   total heap usage: 2 allocs, 1 frees, 1,051 bytes allocated
==950908== 
==950908== 27 bytes in 1 blocks are definitely lost in loss record 1 of 1
==950908==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==950908==    by 0x10919E: get_alphabet_array (main.c:5)
==950908==    by 0x109230: main (main.c:23)
==950908== 
==950908== LEAK SUMMARY:
==950908==    definitely lost: 27 bytes in 1 blocks
==950908==    indirectly lost: 0 bytes in 0 blocks
==950908==      possibly lost: 0 bytes in 0 blocks
==950908==    still reachable: 0 bytes in 0 blocks
==950908==         suppressed: 0 bytes in 0 blocks
==950908== 
==950908== For lists of detected and suppressed errors, rerun with: -s
==950908== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

CODE

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

char* get_alphabet_array(){
    char *alpha =   (char*) malloc(sizeof(char) * 27);

    for(int i = 0; i < 27; i++) {
        if(i == 0) {
            alpha[i] = 'A';
        } else if (i < 26) {
                 alpha[i] =  alpha[i-1] + 1;
        } else if (i == 26) {
            alpha[i] = '\0';
            break;
        }
       //  printf("Character at index %d is: %c\n", i, alpha[i]);
    }
    return alpha;
}

int main () {

    char* alpha = get_alphabet_array();

    while(*alpha != '\0') {
        static int count = 0;
        printf("Letter at index %d: %c\n", count, *alpha);
        count++;
        *alpha++;
    }

    free(*alpha);
    return 0;
}

Solution

  • You started with this

    char* alpha = get_alphabet_array();
    
    while(*alpha != '\0') {
        static int count = 0;
        printf("Letter at index %d: %c\n", count, *alpha);
        count++;
        *alpha++;
    }
    
    free(*alpha);    // <-- this attempts to free what `alpha` points to (not good)
    

    Which, via free(*alpha), attempts to free what alpha points to rather than the pointer alpha. This or course, does not make sense. So you changed it to this:

    char* alpha = get_alphabet_array();
    
    while(*alpha != '\0') {
        static int count = 0;
        printf("Letter at index %d: %c\n", count, *alpha);
        count++;
        *alpha++;   // <-- this alters the value of the pointer, `alpha`
    }
    
    free(alpha);    // <-- this frees the *altered* value of `alpha` (not good)
    

    Now you have a different problem in that the value of alpha has changed since you received it from the memory allocation, and free(alpha) is freeing the wrong pointer value. You need to free the original pointer. For example,

    char* alpha = get_alphabet_array();
    
    for (char *p = alpha; *p != '\0'; p++) {   // this loop does not alter `alpha`
        static int count = 0;
        printf("Letter at index %d: %c\n", count, *p);
        count++;
    }
    
    free(alpha);    // <-- this frees the originally allocated pointer
    

    As an aside, the statement *alpha++; increments the pointer in alpha, then dereferences the pointer and discards the result. So the * here doesn't really serve any purpose or function.