I've created an app to manage students' data which also supports file handling. The Student structure looks like this:

typedef struct Student //dynamically allocated structure
 char *first_name;
 char *last_name;
 float grade;
Student;

Considering C is a low-level programming language, I wanted to take full advantage on its dynamic memory allocation features. That is, both the first name and last name of each student will be allocated dynamically. The main data structure I've used to store the students is a vector. By now, the app has the following features:

• CRUD


• import/export to CSV
  


• import/export to binary file

The file handling part is that interests me the most. I tried to carefully read the C documentation and make my functions less error-prone in terms of memory management and I/O.

The following functions are designed to handle CSV files:

void Read_From_CSV(const char *file_name, Student *p_destination)

 if (!strstr(file_name, ".csv"))
 
 printf("This method only works for CSV files. n");
 return;
 

 FILE *p_file = fopen(file_name, "r");
 if (!p_file)
 
 perror("File not found or corrupted file. n");
 exit(EXIT_FAILURE);
 

 unsigned char buffer_lname[30], buffer_fname[30], buffer[256];
 unsigned int count_students = 0, successfully_read = 0;
 while (fscanf(p_file,"%s",buffer)==1)
 
 successfully_read = sscanf(buffer, "%[^,],%[^,],%f", buffer_lname, buffer_fname, &p_destination[count_students].grade);
 if (successfully_read != 3)
 
 printf("Error reading from CSV.n");
 exit(EXIT_FAILURE);
 

 p_destination[count_students].last_name = (char*)malloc(strlen(buffer_lname) + 1);
 if (!p_destination[count_students].last_name)
 
 printf("Couldn't allocate memory. n");
 exit(EXIT_FAILURE);
 
 strcpy(p_destination[count_students].last_name, buffer_lname);

 p_destination[count_students].first_name = (char*)malloc(strlen(buffer_fname) + 1);
 if (!p_destination[count_students].first_name)
 
 printf("Couldn't allocate memory. n");
 exit(EXIT_FAILURE);
 
 strcpy(p_destination[count_students].first_name, buffer_fname);

 ++count_students;
 
 fclose(p_file);



void Write_To_CSV(const char *file_name, Student *p_source, const unsigned int number_of_students)

 if (!strstr(file_name, ".csv"))
 
 printf("This method only works for CSV files. n");
 return;
 

 FILE *p_file = fopen(file_name, "w");
 if (!p_file)
 
 perror("An error has occured. n");
 exit(EXIT_FAILURE);
 

 unsigned int index = 0;
 int successfully_written = 0;
 while (index < number_of_students)
 

 successfully_written = fprintf(p_file, "%s,", p_source[index].last_name);
 if (successfully_written != strlen(p_source[index].last_name) + 1)
 
 printf("An error has occured. n");
 exit(EXIT_FAILURE);
 
 successfully_written = fprintf(p_file, "%s,", p_source[index].first_name);
 if (successfully_written != strlen(p_source[index].first_name) + 1)
 
 printf("An error has occured. n");
 exit(EXIT_FAILURE);
 
 successfully_written = fprintf(p_file, "%.2f", p_source[index].grade);
 if (successfully_written != 4 && successfully_written != 5)
 
 printf("Error occured during grade writing. n");
 exit(EXIT_FAILURE);
 
 fprintf(p_file, "n");
 ++index;
 
 fclose(p_file);

And these two are designed for binary files:

void Read_From_Binary(const char *file_name, Student *p_destination, const unsigned int number_of_students)

 if (!strstr(file_name, ".bin"))
 
 printf("This method only works for binary files. n");
 return;
 

 FILE *p_file = fopen(file_name, "rb");
 if (!p_file)
 
 perror("Error opening file. n");
 exit(EXIT_FAILURE);
 

 size_t successfully_read;
 unsigned int width = 0;
 for (unsigned int index = 0; index < number_of_students; ++index)
 
 successfully_read = fread(&width, sizeof(width), 1, p_file);
 if (successfully_read != 1)
 
 printf("Error reading from the file. n");
 exit(EXIT_FAILURE);
 

 p_destination[index].last_name = (char*)malloc(width);
 if (!p_destination[index].last_name)
 
 printf("Could not allocate memory. n");
 exit(EXIT_FAILURE);
 
 successfully_read = fread(p_destination[index].last_name, width, 1, p_file);
 if (successfully_read != 1)
 
 printf("Error reading from the file. n");
 exit(EXIT_FAILURE);
 

 successfully_read = fread(&width, sizeof(width), 1, p_file);
 if (successfully_read != 1)
 
 printf("Error reading from the file. n");
 exit(EXIT_FAILURE);
 

 p_destination[index].first_name = (char*)malloc(width);
 if (!p_destination[index].first_name)
 
 printf("Could not allocate memory. n");
 exit(EXIT_FAILURE);
 
 successfully_read = fread(p_destination[index].first_name, width, 1, p_file);
 if (successfully_read != 1)
 
 printf("Error reading from the file. n");
 exit(EXIT_FAILURE);
 

 successfully_read = fread(&p_destination[index].grade, sizeof(float), 1, p_file);
 if (successfully_read != 1)
 
 printf("Error reading from the file. n");
 exit(EXIT_FAILURE);
 
 

 fclose(p_file);


void Write_To_Binary(const char *file_name, Student *p_source, const unsigned int number_of_students)

 if (!strstr(file_name, ".bin"))
 
 printf("This method only works for binary files. n");
 return;
 

 FILE *p_file = fopen(file_name, "wb");
 if (!p_file)
 
 perror("Error opening the file. n");
 exit(EXIT_FAILURE);
 

 size_t successfully_written = 0;
 unsigned int width = 0;
 for (unsigned int index = 0; index < number_of_students; ++index)
 
 width = strlen(p_source[index].last_name) + 1;

 successfully_written = fwrite(&width, sizeof(width), 1, p_file);
 if (successfully_written != 1)
 
 printf("Error writing to the file. n");
 exit(EXIT_FAILURE);
 

 successfully_written = fwrite(p_source[index].last_name, width, 1, p_file);
 if (successfully_written != 1)
 
 printf("Error writing to the file. n");
 exit(EXIT_FAILURE);
 

 width = strlen(p_source[index].first_name) + 1;

 successfully_written = fwrite(&width, sizeof(width), 1, p_file);
 if (successfully_written != 1)
 
 printf("Error writing to the file. n");
 exit(EXIT_FAILURE);
 

 successfully_written = fwrite(p_source[index].first_name, width, 1, p_file);
 if (successfully_written != 1)
 
 printf("Error writing to the file. n");
 exit(EXIT_FAILURE);
 

 successfully_written = fwrite(&p_source[index].grade, sizeof(float), 1, p_file);
 if (successfully_written != 1)
 
 printf("Error writing to the file. n");
 exit(EXIT_FAILURE);
 
 

 fclose(p_file);

UPDATE

As I've been suggested, I'll add a driver code that exemplifies the usage of the earlier defined functions.

Suppose that we have a Students.csv file which contains the following registrations:

Wayne,Bruce,8.50

Kent,Clark,6.60

Dent,Harvey,5.50

Stark,Daniel,7

The driver code will import data from the CSV file. Then it will print the data on the console, write another CSV file and a binary file. The students array is cleared and then we import data from the binary file that was earlier created. Please consider that the structure and the functions for file handling are already defined.

#include <stdio.h>
#include <stdlib.h>

void Print_Students(const Student *p_first, const Student *p_last)

 unsigned int index = 0;
 for (; p_first < p_last; ++p_first)
 
 printf("Student %dn", index);
 printf("Last name: %s n", p_first->last_name);
 printf("First name: %s n", p_first->first_name);
 printf("Grade: %.2f n", p_first->grade);
 ++index;
 
 printf("n");


void main()

 Student *students = (Student*)malloc(4 * sizeof(Student));
 if (!students)
 
 printf("Couldn't allocate memory.n");
 exit(EXIT_FAILURE);
 
 Read_From_CSV("Students.csv", students);
 printf("Reading data from CSV. n");
 Print_Students(students, students + 4);
 Write_To_CSV("Students1.csv", students, 4);
 Write_To_Binary("Students1.bin", students, 4);
 free(students);
 students = (Student*)malloc(4 * sizeof(Student));
 if (!students)
 
 printf("Couldn't allocate memory.n");
 exit(EXIT_FAILURE);
 
 Read_From_Binary("Students1.bin", students, 4);
 printf("Reading data from binary file. n");
 Print_Students(students, students + 4);
 free(students);

My main questions are the following:

1. Are the variables named properly?


2. When working with files, did I correctly check the return values from functions like fscanf, fwrite, fread etc.?


3. Is there a better way to extract data from a CSV file?


4. Is the memory allocation part done properly?

In addition to other good answers:

Allow spaces in names

while (fscanf(p_file,"%s",buffer)==1) stops scanning with any white-space that follows other characters.

// Messes up input
Wayne,Mary Jo,8.50

Better to use fgets(buffer, sizeof buffer, p_file). This also prevents buffer overrun.

Avoid naked magic numbers

Why 30, 256? Use constants to allow for graceful code updates and self documentation.

Be more tolerant of long names. e.g. and their construction should they contain unexpected characters like "'- ,.", etc.

See also Falsehoods Programmers Believe About Names

// unsigned char buffer_lname[30], buffer_fname[30], buffer[256];
#define NAME_LAST_SIZE 64
#define NAME_FIRST_SIZE 60
#define RECORD_FIRST_SIZE (NAME_LAST_SIZE + 1 + NAME_FIRST_SIZE + 1 + 5 + 2];
unsigned char buffer_lname[NAME_LAST_SIZE];
unsigned char buffer_fname[NAME_FIRST_SIZE];
unsigned char buffer[RECORD_FIRST_SIZE * 2]; // I like 2x max expected size

Enforce limits

sscanf(buffer, "%[^,],%[^,],%f" allows buffer overruns leading to undefined bevaior. Consider "%29[^,],%29[^,],%f".

strdup()

The common strdup() or its equivalent would nicely replace repetitive code. Sample code

 // p_destination[count_students].last_name = (char*)malloc(strlen(buffer_lname) + 1);
 // if (!p_destination[count_students].last_name)
 // 
 // printf("Couldn't allocate memory. n");
 // exit(EXIT_FAILURE);
 // 
 // strcpy(p_destination[count_students].last_name, buffer_lname);
 p_destination[count_students].last_name = strdup(buffer_lname);
 if (p_destination[count_students].last_name == NULL) 
 printf("Couldn't allocate memory for last name.n");
 exit(EXIT_FAILURE);
 

Form unique error messages.

Rather than 2 printf("Couldn't allocate memory. n");. Better for error messages to use stderr.

 fprintf(stderr, "Couldn't allocate memory. Line: %dn", __LINE__);
 // or 
 fprintf(stderr, "Couldn't allocate memory last namen");
 fprintf(stderr, "Couldn't allocate memory first namen");

Create file independence.

OP's code makes the file dependent on the size of unsigned and and this code's endianness. Integer size and endian-ness varies amongst platforms.

Instead choose a wide fixed type and convert to a fixed endian, then write. Reading will need to reverse the process.

// unsigned int width = 0;
// width = strlen(p_source[index].last_name) + 1;
// successfully_written = fwrite(&width, sizeof(width), 1, p_file);

uint32_t width = (uint32_t) (strlen(p_source[index].last_name) + 1);
width = htobe32(width);
successfully_written = fwrite(&width, sizeof width, 1, p_file);

I would create a helper function, maybe My_write_uint32(FILE*, uint32_t) to allow code re-use.
Ref: be64toh()

Allocation and deallocation

When allocating memory, don't cast the result of malloc() family. Additionally, we can keep the freedom to change the type of what's allocated if we calculate using sizeof on the result value:

Student *students = malloc(sizeof *students * 4);

We write the sizeof expression first to guarantee that all calculation is done in terms of size_t - that makes no difference here, but is useful when allocating storage for 2-d arrays (malloc(sizeof *array * x * y)).

When we free the students array, it's important to free each individual member first:

free(students); // leaks every first_name and every last_name

I recommend you write a Free_Student() function to do the cleanup, and also one to clean up an entire array.

Filename checking

I consider it an abomination for a data consumer to enforce a file naming policy, but if you insist on having it, you might want to check only the end of the file name for a match - assuming you want data.csv.gz to be a non-matching file:

char const suffix = ".csv";
size_t const suffixlength = sizeof suffix - 1; /* ignoring the NUL */
size_t const filenamelength = strlen(file_name);
if (filenamelength < suffixlength
 || strcmp(suffix, file_name + filenamelength - suffixlength)
 /* handle bad filename */ 

perror doesn't want a newline

Consider

 perror("File not found or corrupted file. n");

It's more informative to show which file is referenced:

 perror(file_name);

This avoids duplicating the information (e.g. "not found", or "permission denied") that perror gives us.

Over-checking of printf() results

In this code, we check the number of characters actually written:

 successfully_written = fprintf(p_file, "%.2f", p_source[index].grade);
 if (successfully_written != 4 && successfully_written != 5)

Really, we're just interested in whether fprintf() was successful or not, and don't mind if we wrote 100.00 (6 chars). So it's better to simply test for the error case, where the function will return a negative value:

 if (fprintf(p_file, "%.2f", p_source[index].grade) < 0)

And we could combine all the prints into one:

 const Student *const p = p_source+index;
 if (fprintf(p_file, "%s,%s,%.2fn",
 p->last_name, p->first_name, p->grade)
 < 0)

Think about const Student where appropriate

The write functions don't need to modify the pointed-to students, so they should take a pointer to const student:

void Write_To_CSV(const char *file_name,
 Student const *p_source,
 const unsigned int number_of_students)

Sanitize your strings

What happens if you have a student with , in their name? Or "? If that's not allowed, then you need to validate at entry and when reading from file. If those are allowed¹, then you'll need to do a bit more formatting to create and read valid CSV. That's an exercise larger in scope than this single paragraph!

¹ You might not have them in the languages of your students, but do be aware that ' is used in some of the Khoisan languages, and that can sometimes require special attention.