where is the leak

[humble edge]

char* getLine(FILE* file) {
    if (file == NULL) return NULL;
    size_t size = 100;
    char* line = malloc(size);
    if (line == NULL) return NULL;
    char c;
    int i = 0;
    while ((c = (char) fgetc(file)) != '\n' && c != EOF) {
        line[i++] = c;
        if (i == size) {
            char* temp = realloc(line, size *= 2);
            if (temp == NULL) {
                free(line);
                return NULL;
            }
            line = temp;
        }
    }

    if (i == 0 && c == EOF) {
        free(line);
        return NULL;
    }

    line[i] = '\0';
    return line;
}

int main() {
    FILE* file = fopen("substantive.txt", "r");
    char* line = getLine(file);
    puts(line);
    fclose(file);
    free(line);
    return 0;
}

CLion tells me that the memory allocated of the function getLine is leaked. but why? i do free the return value?

[cedar lichen]

When your question is answered use !solved to mark the question as resolved.

Remember to ask specific questions, provide necessary details, and reduce your question to its simplest form. For tips on how to ask a good question use !howto ask.

[prime cedar]

Looks fine to me https://godbolt.org/z/drGbv8nxM

Try using sanitizers to tell you where you are leaking and why

[humble edge]

so CLion is just wrong?

[proven horizon]

Looks perfectly fine. It's probably getting tripped up on the realloc-- I've seen that a couple times

[humble edge]

but why is the file always NULL?

int main() {
    FILE* file = fopen("substantive.txt", "r");
    if (file == NULL) {
        puts("File NULL");
        return -1;
    }
    char* line = getLine(file);
    if (line == NULL) {
        puts("line NULL");
        return -1;
    }
    puts(line);
    fclose(file);
    free(line);
  

i have this and it comes always "File NULL" but the file exists which exact that name in the same directory

i mean it says that fopen is deprecated but it should still work right?

[proven horizon]

fopen is not deprecated

Are you executing your program in a different directory?

[humble edge]

ah true the .exe is in cmake-build-debug but the file not

yes i put the text file to the directory where the .exe is and now it works

[proven horizon]

Oh, that's windows pushing their own stuff. Only thing it does differently iirc is check for NULL pointers as parameters

[prime cedar]

To my cursory reading, yes.

But try testing your code and see if you can reproduce a memory leak

Use sanitizers

[humble edge]

did i something wrong? it gave me 0 information

[proven horizon]

It only reports if there was something to report

[humble edge]

int main() {
    FILE* file = fopen("substantive.txt", "r");
    char* line;
    line = getLine(file);
    puts(line);
    return 0;
}

i removed the free on purpose but it still tells nothing

int main() {
    FILE* file = fopen("substantive.txt", "r");
    char* line;
    line = getLine(file);
    puts(line);
    int* ptr = (int*) malloc(10 * sizeof(int));
    free(ptr);
    ptr[3] = 7;
    return 0;
}

hmm now it tells me something

but for just a leak it tells nothing

[oak cobalt]

Excluding this the only reason I could see for clion reporting a memory leak is the free(line) call occurring without checking if line was NULL, which meant you were freeing NULL which is undefined behaviour

[humble edge]

CLion also tells it is leaked when i do the NULL check

[proven horizon]

free(NULL) is defined as a noop. It's not UB

[oak cobalt]

maybe in recent versions of stdlib, never checked that closely. I just know there's places where malloc(0) would return something other than NULL and places where free(NULL) causes a crash and places where realloc(ptr,0) does not free(ptr). So as far as I'm concerned even if it's defined in recent versions of stdlib it's still undefined behaviour due to older versions not being consistent

[proven horizon]

Free(NULL) has been a noop since forever

The free function causes the space pointed to by ptr to be deallocated, that is, made available for further allocation. If ptr is a null pointer, no action occurs
C89 4.10.3.2 The free function

[oak cobalt]

Does that magically make the crashes I've experienced with it somehow disappear? No so UB

[proven horizon]

No, that's not considered UB. I also don't believe whatever you ran into was due to this.
Malloc(0) is not guaranteed to return NULL, but it will return a freeable pointer. Realloc(ptr, 0) does free ptr if it succeeds since realloc frees the old allocation

[oak cobalt]

Debugger directly caught the crash at free(ptr), so definitely due to UB, be it a bug in stdlib or not it's UB at that point

[proven horizon]

A double free, perhaps? I don't believe you unless you can show an actual repro

[oak cobalt]

@humble edge I suggest altering getLine(file) to getLine(file,&line,&size), maybe that will shut clion up, will be faster too

[oak cobalt]

This was years ago, even if I could remember the distro there's no way I'd still remember the version that caused it. TLDR is that learned from then on to always wrap those calls in functions that check for NULL/0 for me to guarantee the behaviour I expect

[humble edge]

CLion is still not satisfied

but how would it be faster? i mean the function does not need to do the initial allocation anymore but now you need to allocate before you use the function so when you see both together would it not be the same?

[proven horizon]

How come you removed the free when realloc fails? That's necessary

[humble edge]

i thought when the function does not allocate the line, then the function is not responsible to free the line

[proven horizon]

Oh I didn't realize you changed how it works

[oak cobalt]

Function can still handle the allocations, *line = malloc/realloc

[humble edge]

then explain how this would be faster

void getLine(FILE* file, char* line, size_t* size) {
    if (file == NULL) return;
    if (line == NULL) return;
    char c;
    int i = 0;
    while ((c = (char) fgetc(file)) != '\n' && c != EOF) {
        line[i++] = c;
        if (i == *size) {
            char* temp = realloc(line, *size *= 2);
            if (temp == NULL) {
                free(line);
                return;
            }
            line = temp;
        }
    }

    if (i == 0 && c == EOF) {
        return;
    }
    realloc(line, i + 1);
    line[i] = '\0';
}

[oak cobalt]

Because you potentially already have memory to work with when calling it

[humble edge]

i did this

[oak cobalt]

Not quite what I meant

give me a mo

size_t getLine(FILE* file, char **line, size_t *size)
{
    if (file == NULL) return 0;
    if (line == NULL) return 0;
    size_t i = 0;
    char *temp = NULL;
    long start_offset = fseek( file, 0, SEEK_CUR );
    while ( fread(file,*line,*size)) != EOF )
    {
        for ( i = 0; i < *size; ++i )
        {
            if ( line[i] == '\n' )
            {
                fseek( file, start_offset + i, SEEK_SET );
                line[i] = 0;
                return i;
            }
        }
        temp = realloc(*line, *size * 2);
        if (temp == NULL)
        {
            free(*line);
            *line = NULL;
            return (size_t)-1;
        }
        fseek( file, start_offset, SEEK_SET );
        *line = temp;
        *size *= 2;
    }

    /* Nothing left to read so release the memory */
    free(line};
    *line = NULL;
    return 0;
}
``` Roughly like that, still some gotta make use of fread's return value but that's the gist of it

[humble edge]

!f

[cedar lichen]

void getLine(FILE* file, char* line, size_t* size) {
  if (file == NULL)
    return;
  if (line == NULL)
    return;
  char c;
  int i = 0;
  while ((c = (char)fgetc(file)) != '\n' && c != EOF) {
    line[i++] = c;
    if (i == *size) {
      char* temp = realloc(line, *size *= 2);
      if (temp == NULL) {
        free(line);
        return;
      }
      line = temp;
    }
  }

  if (i == 0 && c == EOF) {
    return;
  }
  realloc(line, i + 1);
  line[i] = '\0';
}

zxuiji

Formatted by Spectre

[proven horizon]

No, that's no good. The user can't see any reassignment of line

[humble edge]

ah so a pointer to the pointer as parameter

well now it is not leaked anymore 🎉

but still it should work both ways

[proven horizon]

There was no leak in the first place. I've seen other posts here about this sort of realloc in a loop triggering CLion's detection

[humble edge]

void getLine(FILE* file, char** line, size_t* size) {
    if (file == NULL) return;
    if (line == NULL) return;
    char c;
    int i = 0;
    while ((c = (char) fgetc(file)) != '\n' && c != EOF) {
        *line[i++] = c;
        if (i == *size) {
            char* temp = realloc(line, *size *= 2);
            if (temp == NULL) {
                return;
            }
            *line = temp;
        }
    }

    if (i == 0 && c == EOF) {
        return;
    }
    realloc(line, i + 1);
    line[i] = '\0';
}

but this is correct now right?

[proven horizon]

Pass *line to realloc

If you're checking all params for NULL, then you might want to check size as well. You might also want to set *size to be i+1 at the end. Otherwise, yes, this will work with more setup from the caller

[oak cobalt]

@humble edge take another look at the code I edited, I'm done now

I forgot to differentiate between *size == 0 and *size != 0 as well but you get the gist right?

also I shoulda skipped setting i to 0 in the for loop's head but eh, still works as an example

[humble edge]

but why do you set the cursor to the beginning again? then you can only read the first line

[oak cobalt]

the position reset during the loop is so that you read the start of the line, that said I failed to realise I could just do fread( file, *line + i, *size - i ) and continue the loop from i

[humble edge]

ah nevermind

[oak cobalt]

also the fread( file, start_offset + i, SEEK_SET ) shoulda been fread( file, start_offset + i + 1, SEEK_SET ) to put the position just beyond the new line character

[humble edge]

but fread is fread(void* _Buffer, size_t _ElementSize, size_t _ElementCount, FILE* _Stream)?

[oak cobalt]

chalk that up to poor memory and shoddy design on the library's part

Who in their right mind thought putting the data source last was a good idea?

It's not natural in language to say "To Y from X". Even if it does still make sense most people would go with "From X to Y" which is why I end up remembering the order wrong

Doesn't take much to fix the resulting code though

also the call thing in the post you referenced shoulda been fseek, I just typo'd XD

[proven horizon]

I don't see the advantage of using this as an implementation

You've introduced multiple memory errors there

[oak cobalt]

I didn't say it was 100% done, just that it was enough to get the gist of it.

[humble edge]

maybe less io? because i do read everytime a byte from the file with fgetc?

[proven horizon]

The FILE api is buffered. Getting single chars is fine

[oak cobalt]

Entirely possible to implement a getLineCleanup(&line,&size) function to enable keeping hold of the memory longer

[oak cobalt]

Still, making unneccessary calls to fgetc() will be slower than just accessing a buffer directly

Though now that I think of it, what was wrong with fgets()?

[proven horizon]

Implementing a get line function is a common programming exercise I see with C

[oak cobalt]

ah

[humble edge]

it has a fixed sized buffer

i do not like that

[desert carbon]

intel. also every programming language ever, when they made this = that the standard way to assign things

[oak cobalt]

Something wrong with while ( (got = fgets( line + i, max - i, file )) )?

[proven horizon]

Oh, if that's the issue, then you could just call fgets and resize the buffer as needed

[oak cobalt]

you know this = that is vastly different to how people think

[humble edge]

but if the buffer is too small then the line does not fit?

[oak cobalt]

but fair point, I didn't even clock that

[oak cobalt]

That's what the fgets( line + i, max - i, file ) I mentioned addresses

[humble edge]

can you explain that? how can this be used for arbitrary long lines? what is max what is i?

[oak cobalt]

i is the index/offset from line's pointer, max is the current maximum characters or size in this case

size_t max = size / sizeof(char); // Nobody does this since it's same value, only done for wchar_t

I suggest you get used to short hand pseudo code if you're gonna programme because you'll see a LOT of it

[humble edge]

i know pseudocode but i am new to C and still learn the language so it might not be clear to a beginner what you mean with something this short.

could you add how you would allocate and reallocate the line? also what if the line is twice as long as the initial max?

[oak cobalt]

sure, give me a mo

char *pos = line;
uintptr_t i = 0;
while ( (pos = fgets( pos, size - i, file )) )
{
   i = (uintptr_t)(pos - line);
   if ( i < size )
      break;
   temp = realloc( line, size * 2 );
   if ( temp )
   {
      line = temp;
      size *= 2;
      continue;
   }
}
// We have the whole line now, carry on to do our logic
...

This example doesn't account for failures to grab the line at all so do account for that when you start adding to it

ah, forgot to increment i - corrected now, along with adding in the check for getting no characters at all

[humble edge]

ah that makes sense. thank you.

[proven horizon]

fgets returns a pointer to your allocation, not the amount read

[oak cobalt]

ah, I forgot that detail

[humble edge]

well but then it is not possible right?

[oak cobalt]

Code should be correct now, unless I didn't consider something again and knowing me that's quite probable

[cedar lichen]

@humble edge Has your question been resolved? If so, type !solved :)

[humble edge]

but it does not work. i also added i += instead of i = but it does not read the whole line

[oak cobalt]

That'll be because I didn't remember this: If bytes are read and no errors occur, writes a null character at the position immediately after the last character written to str

In other words I was checking that pos equaled line + size with my example when I shoulda checked it equalled line + (size - 1)

I'm gonna go sleep now, here's a link for https://en.cppreference.com/w/c/io/fgets from a decent site to reference when coding with the standard library and C in general

[humble edge]

good night

!solved