Issue with File IO in C

Question

I am trying to read a file every time the user inputs 1 but i am facing an unexpected behaviour,

This is my code.

#include <stdio.h>
#include <string.h>

char* read_file(char* file_name){

    FILE *fp = fopen(file_name, "r");

    if (fp == NULL)
    {
        printf("Error: could not open file %s", file_name);
        return "";
    }

    // reading line by line, max 256 bytes
    const unsigned MAX_LENGTH = 30000;
    char buffer[30000];
    char data[MAX_LENGTH];
    memset(buffer, '\0', MAX_LENGTH);
    while (fgets(buffer, MAX_LENGTH, fp)){
        strcat(data, buffer);
        // strcat(data, "\n");
    }
    // close the file
    fclose(fp);
    char* return_data = &data[0];
    return return_data;

}

void send_data(char* file_name){
    printf("\n\n %s\n", read_file(file_name));
}

int main()
{
    int cont = 1;
    while(cont){
        send_data("try/try.txt");
        scanf("%d", &cont);
    }
}

What is happening is when it is giving the output the first time it gives the correct result but on the second time it adds the prev result and new result and prints it. Is there any way i can correct it.

Here is the output log

tusharsing@techie-Latitude-3520:~/Desktop/Project/project/temp$ ./filetry 


 HTTP/1.1 200 OK
Content-Type: text/plain
Content-Length: 12

<!DOCTYPE html>
<html>
<head>
<title>Page Title</title>
</head>
<body>

<h1>My First Heading</h1>
<p>My first paragraph.</p>

</body>
</html>

1


 HTTP/1.1 200 OK
Content-Type: text/plain
Content-Length: 12

<!DOCTYPE html>
<html>
<head>
<title>Page Title</title>
</head>
<body>

<h1>My First Heading</h1>
<p>My first paragraph.</p>

</body>
</html>
HTTP/1.1 200 OK
Content-Type: text/plain
Content-Length: 12

<!DOCTYPE html>
<html>
<head>
<title>Page Title</title>
</head>
<body>

<h1>My First Heading</h1>
<p>My first paragraph.</p>

</body>
</html>

^C

`strcat(data, buffer);` but `data` is uninitialised. OTOH the `buffer` is, but does need to be, because you've read a C string into it. The result is undefined, possibly erratic, behaviour. — Weather Vane, Jun 05 '23 at 21:38
I'm afraid that it only appears to work. It doesn't really work because the lines `char* return_data = &data[0];` and `return return_data;` are returning a pointer to a local variable. That results in undefined behavior. — user3386109, Jun 05 '23 at 22:03
@TusharSingh Why the comment `// reading line by line, max 256 bytes`, yet read up to 30000 per line? — chux - Reinstate Monica, Jun 06 '23 at 00:02
Even after fixing the lack of initialization for `data`, the loop `while (fgets(buffer, MAX_LENGTH, fp)) strcat(data, buffer);` will overflow `data` if reading a file greater than ~30kB. This needs bounds checking / a different approach to avoid this possibility. — Oka, Jun 06 '23 at 02:22

Andreas Wenzel · Accepted Answer · 2023-06-06T07:23:39.483

The function call

strcat(data, buffer);

requires that data points to a null-terminated string. However, this is not necessarily the case in the first iteration of the loop, because you are not ensuring that data constains a null terminating character.

If you change the line

memset(buffer, '\0', MAX_LENGTH);

to

memset(data, '\0', MAX_LENGTH);

then you are ensuring that the first character is a null character (which makes the length of the string zero).

However, in order to set the first character of the string to a null character, it would be sufficient to write:

data[0] = '\0';

There is no need to set the entire string to zero.

Another issue is that the line

return return_data;

does not make sense, because return_data has the value &data[0], and data is a local array. This means that the return value is a dangling pointer. It points to an object that no longer exists as soon as the function returns. Therefore, such a pointer is useless.

If dereferencing the dangling pointer happens to work, then that just means that you are lucky that the memory location of the expired object has not yet been overwritten. You cannot rely on this behavior. See this answer to another question for more information on why this happens and why you cannot rely on it.

If you want an array that is still accessible when the function returns, then either

the caller will have to allocate the memory for the array and pass a pointer to that array to the called function, or
the called function will have to use dynamic memory allocation (e.g. malloc)

Issue with File IO in C

1 Answers1