realloc() is wasting a lof of space, what did I do wrong?

Question

So, I was doing this exercise:

Write a C function void occurrences(char* s, char c, char*** occp, int* n) that, given a string s and a char c, counts the number of occurrences of char c in the string s, returns that number in n and returns in occp the adress of a new array of char that contains the adresses of each c occurrence in s

main sample:

#include <stdio.h>

int main(){
    int i, n;
    char** occ;
    occorrenze("engineering", 'n', &occ, &n);
    for (i=0; i<n; ++i) printf("%s\n", occ[i]); // prints ngineering neering ng
    free(occ);
}

Initially I wrote the function this way:

void occurrences(char* s1, char c, char*** s, int* n){
    *n=0;
     char* arr[2];
     int length=strlen(s1);
     int i;
     for(i=0; i<length; i++){
        if(s1[i]==c)(*n)++;
     }
     *s=(malloc((*n)*sizeof(char**)));
     int a=0;
     for(i=0; i<length; i++){
        if(s1[i]==c){
           (*s)[a]= &s1[i];
           a++;
        }
     }
}

Worked well, but I wanted to try and re-write it iterating the string just one time. I thought of using realloc(), function which I never used before and eventually I came up with this:

void occurrences(char* s1, char c, char*** s, int* n){
    *n=0;
    *s=malloc(0);
     char* arr[2];
     int length=strlen(s1);
     int i,a=0;
     for(i=0; i<length; i++){
        if(s1[i]==c){
            (*n)++;
            *s=realloc(*s,(*n)*sizeof(char**));
            (*s)[a]= &s1[i];
            a++;

        }
     }
}

This one seemed to work well too, but then I run Valgrind:

==4893== HEAP SUMMARY:
==4893==     in use at exit: 0 bytes in 0 blocks
==4893==   total heap usage: 4 allocs, 4 frees, 48 bytes allocated
==4893== 
==4893== All heap blocks were freed -- no leaks are possible
==4893== 
==4893== For counts of detected and suppressed errors, rerun with: -v
==4893== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)

48 bytes allocated ? It should've been 24 bytes, right ? The total heap size is 8*n! instead of 8*n... I guess I am missing something XD

EDIT: copied the right function lol

32-bit or 64-bit system? 64-bit systems use 8-byte pointers, which would make 48. — DoxyLover, May 21 '14 at 19:45
This might not be what you're hoping for, but honestly it would be more efficient to do this your original way. You are writing a function that should work for all strings, I suppose, so you don't have any statistics on input size to find a way to make a reallocation scheme more efficient than simply counting up how big you need to make your result string and allocating space accordingly. Lastly, the processor will likely have branch prediction, making repeated loops more efficient. — Joel Dentici, May 21 '14 at 19:49
It's not going to change the outcome here, but technically you should be alloc'ing `(*n)*sizeof(char*)` bytes (not using `sizeof(char**))`. — nobody, May 21 '14 at 19:50
@user2482551: Thanks for your explaination ! BTW I wasn't trying to make the function more efficient, I was just "experimenting" :) — Atlas80b, May 21 '14 at 19:58

score 4 · Answer 1 · edited May 21 '14 at 19:47

4

Doesn't valgrind measure total allocated memory throughout the application's execution?

0 + 8 + 16 + 24 = 48.

edited May 21 '14 at 19:47

nobody

19,814
17
56
77

answered May 21 '14 at 19:47

Mark

76
3

I just discovered I was misunderstanding valgrind's output.. thanks XD – Atlas80b May 21 '14 at 20:01

Mahonri Moriancumer · Accepted Answer · 2014-05-21T20:13:22.550

HEAP USAGE:

If you malloc(1), the system allocates a chunk of memory for you that is capable of holding one byte. What you might not realize though, the system would generally never allocate a single byte. In reality, the allocated chunk of memory is most likely much larger; perhaps a minimum of 8, 16 or 32 bytes. Hence, malloc()ed space does not equal heap space-used.

And due to the above 'minimum' system memory allocations, the function realloc() may return the same address as it was given; if the system allocation at that address is large enough to accomidate the (in your case) larger size. Once the system allocation becomes too small, realloc() will return a new address to a larger chunk of system memory (and most likely larger than requested by realloc).

A few comments:

void occurrences(char* s1, char c, char*** s, int* n){
   *n=0;
   *s=NULL;

Changed the above line. No need to malloc(0);. The value of s (NULL) will be passed in to realloc(), which then works just like malloc(),

    char* arr[2];
    int length=strlen(s1);
    int i,a=0;
    for(i=0; i<length; i++){
    if(s1[i]==c){
        (*n)++;
        *s=realloc(*s,(*n) * sizeof(char**));  // !DANGER!

Instead of the above line, consider the following replacement:

    if(s1[i]==c){
        char *tmp;
        (*n)++;
        tmp=realloc(*s,(*n) * sizeof(char**));  // Much better.
        if(NULL == tmp)
           {
           /* Handle realloc() failure. */
           ...
           }
        else
           *s = tmp;  

        ...

If you don't do it as shown above, if realloc() fails, the previously allocated memory (pointed to by s) is lost; replaced by NULL.

        (*s)[a]= &s1[i];
        a++;

    }
 }

}

score 0 · Answer 3 · answered Jul 17 '21 at 13:38

0

You can save yourself major headaches if you don’t use *n and *occp as variables, but use local variables and store them at the very end of the function. Makes things a lot clearer and reduces the chances if bugs.

answered Jul 17 '21 at 13:38

gnasher729

51,477
5
75
98

realloc() is wasting a lof of space, what did I do wrong?

3 Answers3