I know the theory of fwrite and fread but I must be making some mistake because I can't make them work. I made a random struct, initialized an array and used fwrite to save it into a binary file. Then I opened the same binary file, used fread and saved what was inside in another array. With the debugger I saw what was inside the second array and it says, for example:
ParkingLot2[0].company= "ffffffffffffffff"
ParkingLot2[0].years=-842150451.
When pasting the code I removed all the stuff that made the code too long like if (f==NULL) for controlling the opening of the file and the pointer==NULL after the mallocs and the control that fread and fwrite read the right amount of data.
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
typedef struct Cars {
int years;
char company[16];
}Tcars;
void WriteBinaryFile(char* file_name, Tcars* p_1)
{
FILE* f;
f = fopen(file_name, "wb");
fwrite(p_1, sizeof(Tcars), 3, f);
fclose(f);
return;
}
Tcars* ReadBinaryFile(char* file_name, Tcars* p_2, int* pc)
{
FILE* f;
Tcars temp;
size_t number = 1;
f = fopen("cars.dat", "rb");
while (number) {
number = fread(&temp, sizeof(Tcars), 1, f);
if (number)
(*pc) ;
}
/*i already know that the size is 3 but i want to try this method because in my last exam i
was given a .dat file from my professor and i didn't know how much data i had to read through */
if ((*pc) != 0)
{
p_2 = malloc(sizeof(Tcars) * (*pc));
fread(p_2, sizeof(Tcars), (*pc), f);
}
fclose(f);
return p_2;
}
int main()
{
Tcars* ParkingLot1 = malloc(sizeof(Tcars) * 3);
for(int i=0;i<3;i )
{
ParkingLot1[i].years = 2000 i;
}
strcpy(ParkingLot1[0].company, "Fiat");
strcpy(ParkingLot1[1].company, "Ford");
strcpy(ParkingLot1[2].company,"Toyota");
Tcars* ParkingLot2 = NULL;
int cars_amount = 0;
WriteBinaryFile("cars.dat", ParkingLot1);
ParkingLot2 = ReadBinaryFile("cars.dat", ParkingLot2, &cars_amount);
free(ParkingLot1);
free(ParkingLot2);
return 0;
}
CodePudding user response：
You have a number of small (and some not so small errors) that are causing you problems. Your primary problem is passing Tcars* p_2 to ReadBinaryFile() and allocating with p_2 = malloc(sizeof(Tcars) * (*pc)); each time. Why?
Each call to malloc() returns a new block of memory with a new and different address. You overwrite the address of p_2 with each new call, creating a memory leak and losing the data stored prior to the last call. Instead, you need to realloc() to reallocate a larger block of memory and copy your existing data to the new larger block so you can add the next Tcars worth of data at the end of the reallocated block.
If you are reading data from one file, then there is no need to pass p_2 as a parameter to begin with. Simply declare a new pointer in ReadBinaryFile() and realloc() and add each Tcars worth of data at the end and then return the newly allocated pointer. (you must validate every allocation and reallocation)
Your choice of void for WriteBinaryFile() will conceal any error encountered creating or writing to your new file. You must validate EVERY input/output file operation, especially if the data written will be used later in your program. A simple choice of return of type int returning 0 for failure and 1 for success (or vice-versa, up to you) is all you need. That way you can handle any error during file creation or writing and not blindly assume success.
A slightly more subtle issue/error is your allocation for ParkingLot1 using malloc(). Ideally you would use calloc() or use memset to set all bytes zero. Why? You write the entire Tcars struct to the file. malloc() does not initialize the memory allocated. That means all characters in the company name between the end of the name (the nul-terminating character) and the end of the 16 bytes of storage will be uninitialized. While that will not cause problems with your read or write, it is far better to ensure all data being written to your file is initialized data. Otherwise examining the contents of the file will show blocks of uninitialized values written to the file.
Another small style issue is the '*' in the declaration of a pointer generally goes with the variable and not the type. Why?
Tcars* a, b, c;
The declaration above most certainly does not declare 3-pointers of type Tcars, instead it declares pointer a and two struct of type Tcars with automatic storage duration b, and c. Writing:
Tcars *a, b, c;
makes that clear.
Lastly, don't use MagicNumbers or hardcode filenames in your functions. You shouldn't need to recompile your code just to read or write a different filename. It's fine to use "cars.dat" as a default filename in main(), but either take the filenames as the 1st argument to your program (that's what int argc, char **argv parameters to main() are for) or prompt the user for a filename and take it as input. 3 and 16 are MagicNumbers. If you need a constant, #define them or use a global enum.
Putting it altogether, you could do something similar to the following to read an unknown number of Tcars from your data file:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define NCARS 3 /* if you need a constant, #define one (or more) */
#define COMPANY 16
typedef struct Cars {
int years;
char company[COMPANY];
} Tcars;
/* returns 1 on success 0 on error */
int WriteBinaryFile (char *file_name, Tcars *p_1, size_t nelem)
{
FILE *f;
size_t size = sizeof *p_1;
f = fopen (file_name, "wb");
if (!f) { /* validate file open for writing */
perror ("fopen-file_name-write");
return 0;
}
/* validate that nelem blocks of size are written to file */
if (fwrite (p_1, size, nelem, f) != nelem) {
return 0;
}
if (fclose (f)) { /* validate close-after-write */
perror ("fclose-f");
return 0;
}
return 1;
}
/* returns pointer to allocated block holding ncars cars on success,
* NULL on failure to read any cars from file_name.
*/
Tcars *ReadBinaryFile (char *file_name, size_t *ncars)
{
FILE *f;
Tcars *tcars = NULL, temp;
size_t nelem = *ncars, size = sizeof (Tcars);
f = fopen (file_name, "rb");
if (!f) {
perror ("fopen-file_name-read");
return NULL;
}
while (fread (&temp, size, 1, f) == 1) {
/* always realloc to a temporary pointer */
void *tempptr = realloc (tcars, (nelem 1) * size);
if (!tempptr) { /* validate realloc succeeds or handle error */
perror ("realloc-tcars");
break;
}
tcars = tempptr; /* assign reallocated block */
memcpy (tcars nelem, &temp, size); /* copy new car to end of block */
nelem = 1;
}
fclose (f);
*ncars = nelem;
return tcars;
}
/* void is fine for print functions with no bearing on the
* continued operation of your code.
*/
void prn_cars (Tcars *cars, size_t nelem)
{
for (size_t i = 0; i < nelem; i ) {
printf ("M %s\n", cars[i].years, cars[i].company);
}
}
int main (int argc, char **argv)
{
/* read from filename provided as 1st argument ("cars.dat" by default) */
char *filename = argc > 1 ? argv[1] : "cars.dat";
/* must use calloc() on ParkingLot1 or zero memory to avoid writing
* unintialized characters (rest of company) to file.
*/
Tcars *ParkingLot1 = calloc (NCARS, sizeof(Tcars)),
*ParkingLot2 = NULL;
size_t cars_amount = 0;
if (!ParkingLot1) { /* validate EVERY allocation */
perror ("calloc-ParkingLot1");
return 1;
}
for (int i = 0; i < NCARS; i ) {
ParkingLot1[i].years = 2000 i;
}
strcpy (ParkingLot1[0].company, "Fiat");
strcpy (ParkingLot1[1].company, "Ford");
strcpy (ParkingLot1[2].company,"Toyota");
/* validate WriteBinaryFile succeeds or handle error */
if (!WriteBinaryFile (filename, ParkingLot1, NCARS)) {
return 1;
}
ParkingLot2 = ReadBinaryFile (filename, &cars_amount);
if (ParkingLot2) { /* validate ReadBinaryFile succeeds or handle error */
prn_cars (ParkingLot2, cars_amount); /* output cars read from file */
free (ParkingLot2); /* free if ParkingLot2 not NULL */
}
free(ParkingLot1); /* free ParkingLot1 */
}
(note: you always check the return of fclose() after-a-write to catch any file errors and error flushing the data to the file that can't be caught at the time of the fwrite() call)
Also note in the man page for fread and fwrite they can read or write less than the number of byte (or elements) you request. A short-read may or may not represent an error or premature end-of-file and you need to call ferror() and feof() to determine which, if any, occurred. While direct file reads from disk are not as prone to short-reads as network reads and writes, a full implementation would protect against a short-read regardless of where the data is being read from or written to. Further investigation is left to you.
Example Use/Output
$ ./fwrite_fread_cars dat/cars.dat
2000 Fiat
2001 Ford
2002 Toyota
Memory Use/Error Check
In any code you write that dynamically allocates memory, you have 2 responsibilities regarding any block of memory allocated: (1) always preserve a pointer to the starting address for the block of memory so, (2) it can be freed when it is no longer needed.
It is imperative that you use a memory error checking program to ensure you do not attempt to access memory or write beyond/outside the bounds of your allocated block, attempt to read or base a conditional jump on an uninitialized value, and finally, to confirm that you free all the memory you have allocated.
For Linux valgrind is the normal choice. There are similar memory checkers for every platform. They are all simple to use, just run your program through it.
$ valgrind ./fwrite_fread_cars dat/cars.dat
==7237== Memcheck, a memory error detector
==7237== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==7237== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==7237== Command: ./fwrite_fread_cars dat/cars.dat
==7237==
2000 Fiat
2001 Ford
2002 Toyota
==7237==
==7237== HEAP SUMMARY:
==7237== in use at exit: 0 bytes in 0 blocks
==7237== total heap usage: 9 allocs, 9 frees, 10,340 bytes allocated
==7237==
==7237== All heap blocks were freed -- no leaks are possible
==7237==
==7237== For lists of detected and suppressed errors, rerun with: -s
==7237== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
Always confirm that you have freed all memory you have allocated and that there are no memory errors.
Look things over and let me know if you have any question.