Please don't cast malloc
.
You allocating the wrong number of bytes, sizeof(char)
returns you the size of
a single char
, so you are allocating one byte only. That's not enough to hold
a string.
You should use malloc
like this:
int *a = malloc(100 * sizeof *a);
if(a == NULL)
{
// error handling
// do not continue
}
Using sizeof *a
is better than sizeof(int)
, because it will always return
the correct amount of bytes. sizeof(int)
will do that as well, but the problem
here is the human factor, it's easy to make a mistake and write sizeof(*int)
instead. There are thousands of questions here with this problem.
Note that a char
is defined to have the size of 1, that's why when you are
allocating memory for strings or char
arrays, people usually don't write the
* sizeof *a
:
char *s = malloc(100);
// instead of
char *s = malloc(100 * sizeof *s);
would be just fine. But again, this is only the case for char
. For other types
you need to use sizeof
operator.
You should always check the return value of malloc
, because if it returns
NULL
, you cannot access that memory.
while (fscanf(f, "%c", &s[n]) != EOF)
n++;
If you for example allocated 100 spaces, you have to check that you haven't
reached the limit. Otherwise you will overflow s
:
char *s = malloc(100);
if(s == NULL)
{
fprintf(stderr, "not enough memory\n");
return 1;
}
int n = 0;
while ((fscanf(f, "%c", &s[n]) != EOF) && n < 100)
n++;
In this case you are not using the allocated memory to store a string, so it's
fine that it doesn't have the '\0'
-terminating byte. However, if you want to
have a string, you need to write one:
while ((fscanf(f, "%c", &s[n]) != EOF) && n < 99)
n++;
s[n] = '\0';
Also you are doing this
free(s);
for (i=0;i<n;i++)
printf("%c",s[i]);
You are freeing the memory and then trying to access it. You have to do the
other way round, access then free.
Correct way:
for (i=0;i<n;i++)
printf("%c",s[i]);
free(s);
EDIT
If you want the contents of a whole file in a single string, then you have 2
options:
- Calculate the length of the file beforehand and then use allocate the correct
amount of data
- Read one fixed size chunk of bytes at a time and resize the memory every time
you read a new chunk.
The first one is easy, the second one is a little bit more complicated because
you have to read the contents, look how much you've read, resize the memory with
realloc
, check that the resizing was successful. This is something you could
do later when you have for knowledge in simple memory managment.
I'll show you the first one, because it's much easier. The function fseek
allows you to advance your file pointer to the end of the file, with the
function ftell
you can get the size of the file and with rewind
rewind the file pointer and set it to the beginning:
#include <stdio.h>
#include <stdlib.h>
int main(int argc, char **argv)
{
if(argc != 2)
{
fprintf(stderr, "usage: %s file\n", argv[0]);
return 0;
}
FILE *fp = fopen(argv[1], "r");
if(fp == NULL)
{
fprintf(stderr, "Could not open %s for reading.\n", argv[1]);
return 1;
}
// calculating the size
// setting file pointer to the end of the file
if(fseek(fp, 0L, SEEK_END) < 0)
{
fprintf(stderr, "Could not set the file pointer to the end\n");
fclose(fp);
return 1;
}
// getting the size
long size = ftell(fp);
if(size < 0)
{
fprintf(stderr, "Could not calculate the size\n");
fclose(fp);
return 1;
}
printf("file size of %s: %ld\n", argv[1], size);
// rewinding the file pointer to the beginning of the file
rewind(fp);
char *s = malloc(size + 1); // +1 for the 0-terminating byte
if(s == NULL)
{
fprintf(stderr, "not enough memory\n");
fclose(fp);
return 1;
}
int n = 0;
// here the check && n < size is not needed
// you allocated enough memory already
while(fscanf(fp, "%c", &s[n]) != EOF)
n++;
s[n] = '\0'; // writing the 0-terminating byte
fclose(fp);
printf("Contents of file %s\n\n", argv[1]);
for(int i=0; i<n; i++)
printf("%c",s[i]);
free(s);
return 0;
}