malloc causes

Question

I am trying to read a file line by line, and get each line as a char * to a dynamic string, the code I am using used to work and without changing it (or noticing it), it has ceased to work, accsesing the reed information results in an error. Here is a MRE of my code for getting one line:

#include <stdio.h>
#include <string.h>
 
#define MAX_STR_SIZE 10000

int main(void)
{
char* filePath; // is set by other working part of program to a real readable file address.
    while (fgetc(filePath) != EOF) // an extra chracter is in the data files to account for this cheack.
    {
        char tempStr[MAX_STR_SIZE] = { 0 };
        char* str = NULL;
        fgets(tempStr, MAX_STR_SIZE, filePath);
        tempStr[strcspn(tempStr, "\n")] = 0;
        str = malloc(sizeof(char) * (strlen(tempStr) + 1)); // does not work
        strcpy(str, tempStr); 
    }
}

The error: Exception thrown at 0x00007ff95448d215 in GifProject.exe: Access violation writing location 0xFFFFFFFFEA1854F0.

You don't need (and should not) cast the return value of `malloc`. Your allocated size does not make sense. You want to copy a string, not pointers. That line should be `str = malloc(strlen(tempStr)+1);` You missed the `+1` for the terminating 0 byte. But as you multiplied by `sizeof(str)` instead of `sizeof(*str)` (which is always 1) you allocated more memory than you need. — Gerhardh, Jun 25 '22 at 19:47
You should generally check return values value of `malloc` if it is `NULL`. But this does not seem to be a problem here. — Gerhardh, Jun 25 '22 at 19:48
If `strlen(tempStr)` was `0` that will cause an error, because 0 bytes will be allocated, but it needs room for a string terminator. — Weather Vane, Jun 25 '22 at 19:53
Please create a [mcve]; the bug may be in some other part of your code. And your error message should be posted as text (copy and paste), not as an image. — Nate Eldredge, Jun 25 '22 at 19:55
Interstingly, if you try to allocate 0 bytes [*either NULL or a pointer suitable to be passed to free() is returned*](https://man7.org/linux/man-pages/man3/malloc.3.html). From your screenshot your pointer is not `NULL` but you still might get a pointer that you may not access. — Gerhardh, Jun 25 '22 at 19:58
The original message in the question title `` which I edited is what the IDE has shown for the value of `str`. That is not the string content: it indicates that the pointer assigned to `str` cannot be dereferenced. But strangely, `tempStr` is `"5"` not `""`. — Weather Vane, Jun 25 '22 at 20:05
You should put a breakpoint before that line and check your variables. After that exception was reported, some contents in the debugger might no longer be correct. — Gerhardh, Jun 25 '22 at 20:18
@Gerhardh, I fixed the malloc caculation, as you pointed out, and tested the code with stdin incase the file was the problem, however, I am still getting the same result. Also, this may be a dumb question, but why should I not cast the return value of malloc? And what should I do instead? — Yoav Raman, Jun 25 '22 at 20:37
@Gerhardh Also, I checked if the values are the same before the break point and they are. — Yoav Raman, Jun 25 '22 at 20:39
If `fgets` hits EOF on `file` (which you do _not_ check for the `NULL` return value) then `tempStr` will have 0x00 as the first byte. `strlen` will return 0 and the arg to `malloc` will be 0. As Gerhardh mentioned change to: `strlen(tempStr)+1` to handle this issue. Or, just do: `str = strdup(tempStr);` — Craig Estey, Jun 25 '22 at 20:41
It's here: [Do I cast the result of malloc?](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) — Weather Vane, Jun 25 '22 at 20:41
But if you get EOF the function should return `NULL` and the _caller_ should check for this and exit _its_ loop — Craig Estey, Jun 25 '22 at 20:45
Your refusal to show the requested MRE forced everyone into guessing and made the error really hard to find. — Gerhardh, Jun 25 '22 at 23:10

chqrlie · Accepted Answer · 2022-06-25T21:34:03.803

4

It is difficult to diagnose your problem without a complete compilable program that exhibits the problem, but from the code fragment and the debugging information in the image, it seems you do not include <stdlib.h> and the prototype inferred by the compiler for malloc() from the actual argument is int malloc(size_t), leading to undefined behavior when you store the return value into the pointer str: because of the missing prototype, the compiler generates code that converts the return value from int to char *, sign extending from 32-bit to 64-bits, producing a meaningless pointer.

Note that you should also test the return value of fgets to properly handle end of file, and you should test for potential malloc failure before calling strcpy or better: use strdup that allocates and copies a string in a single call.

Here is a modified version:

#include <stdlib.h>
#include <string.h>

#define MAX_STR_SIZE  4096

char *readline(FILE *file) {
    char tempStr[MAX_STR_SIZE];
    if (!fgets(tempStr, sizeof tempStr, file)) {
        /* end of file: return a null pointer */
        return NULL;
    }
    /* strip the trailing newline if any */
    tempStr[strcspn(tempStr, "\n")] = '\0';
    /* allocate a copy of the string and return it */
    return strdup(tempStr);
}

edited Jun 25 '22 at 21:34

answered Jun 25 '22 at 21:02

chqrlie

131,814
10
121
189

1

Are you sure that not including stdlib makes malloc allocate pointers to 'int' by default ? – Yvain Jun 25 '22 at 21:23
3

@Yvain: not at all. The `malloc()` function returns a pointer, calling it without a proper prototype has undefined behavior. My guess if that without ``, `malloc()` does not have a defined prototype, hence the compiler, in a lax compatibility mode, infers a prototype from the actual arguments and defaults the return type to `int`. The value of `str` in the debug screenshot is displayed as `0xffffffffea1854f0`, which is a 64-bit address with all 33 upper bits set, typical of converting a 32-bit `int` to a 64-bit pointer. The actual return value was modified as if by `(char*)(int)v` – chqrlie Jun 25 '22 at 21:31
I am used to calls to malloc and never had to cast the return of malloc when wanting chars. I just guess that is the default behaviour for c on linux. – Yvain Jun 25 '22 at 21:35
2

@Yvain nobody is suggesting to cast the return value of `malloc`. Quite the opposite. If you missed the `stdlib.h` thing and had sufficient warnings/errors enabled, the compiler would have flagged the statement as trying to convert an `int` to a pointer. Casting the return might mask this. – Craig Estey Jun 25 '22 at 21:51
Good catch! The valid address in the screenshot has the upper nibbles of the lower 32 bits containing `ea17`. The invalid address has `ea18` which is similar range. With that erronous conversion from int to pointer the sign bit gets extended resulting in that strange address. Yet another example why complete example are mandatory! – Gerhardh Jun 25 '22 at 23:08
Thank you! I have learend alot from everybodies answers, both about coding, and mostly about stackOverFlow, the problem was indeed what you pointed out in the first paregraph! – Yoav Raman Jun 28 '22 at 17:09

malloc causes

1 Answers1