There are several problems in your code. First of all, in the while loop, you need to test if i
is greater than MAX
, in which case you need to exit the loop to avoid buffer overflow. As it is now, if the file is longer than 1024 characters, it will crash because you're trying to write outside of the array. To avoid this, add this code to the while loop (after i++
):
if(i + 1 >= MAX){
break;
}
The break;
command will exit the while loop.
Second of all, strings have to end with a null character. So you have to add a null character to each string after the while loop like this:
rez[i] = '\0';
buf[i] = '\0';
Note that when testing if i
is greater than MAX
, we did if(i + 1 >= MAX)
instead of if(i >= MAX)
to leave space for the null character.
Third of all, never use malloc
without free
, otherwise it will cause a memory leak. You have to free rez
and buf
at the end of your program like this:
free(rez);
free(buf);
An alternative to this which works in your case but not always would be to declare rez
and buf
as simple arrays without using malloc
:
char buf[MAX];
char rez[MAX];
This works since MAX
is a macro and not a variable.
Fourth of all, never leave a pointer uninitialized. Instead, initialize it to NULL
:
char *buf = NULL;
However, in your case, it would be better to initialize it to malloc
directly:
char *buf = (char*)malloc(sizeof(char) * MAX);
Note that in C, it is not required nor recommended to cast the result of malloc
(although it is required in C++). The following code would therefore be better:
char *buf = malloc(sizeof(char) * MAX);
For more information, you can read this.
Fifth of all, as suggested in the comment, you have to check if the memory allocation from malloc
failed, in which case you need to exit the program to avoid undefined behavior. If the memory allocation failed, the return value of malloc
will be NULL
. That gives this code, that you should place after malloc
:
if(rez == NULL || buf == NULL){
printf("Memory allocation failed.\n");
exit(8);
}
The return value in this case would be 8 if you want to follow the standard return values. Of course, you can return what ever value you want if you don't want to follow the standard. If you want to follow the standard, the return value should also be 2 if the file isn't found (not 1 as in your code). You can also use the macros ERROR_FILE_NOT_FOUND
(equal to 2) and ERROR_NOT_ENOUGH_MEMORY
(equal to 8). These macros can be found in windows.h
(although I don't know if you have this file since you're using Linux).
A version of your code that works would then be:
#include <stdio.h>
#include <stdlib.h>
#define MAX 1024
int main(int argc, char **argv){
FILE *f1;
char *buf = malloc(sizeof(char) * MAX);
char *rez = malloc(sizeof(char) * MAX);
if((f1=fopen("fis4.txt", "r" )) == NULL){
printf("Error\n");
exit(2);
}
if(rez == NULL || buf == NULL){
printf("Memory allocation failed.\n");
exit(8);
}
int i = 0;
while((buf[i] = fgetc(f1)) != EOF){
rez[i] = buf[i];
i++;
if(i + 1 >= MAX){
break;
}
}
rez[i] = '\0';
buf[i] = '\0';
printf("%s", rez);
free(rez);
free(buf);
fclose(f1);
return 0;
}