I am new to C programming. I am trying to write a C struct to a file and read it back. I am using valgrind to check for memory leaks and potential errors. I get a set of valgrind errors when I run the program and no memory leaks. The program also accurately writes the data to the file and reads the data back from the file. I created a separate test program (testRead) to test reading back the file after the test program for writing the file (testUsers) is run, and the data is read back correctly. I also can verify the data is correct on disk using hexdump. What do the valgrind errors mean - what are the uninitiated bytes referenced in the error message?
The error returned by running: valgrind -s --track-origins=yes --leak-check=full --show-leak-kinds=all ./testUsers
==1155442== HEAP SUMMARY:
==1155442== in use at exit: 0 bytes in 0 blocks
==1155442== total heap usage: 11 allocs, 11 frees, 10,448 bytes allocated
==1155442==
==1155442== All heap blocks were freed -- no leaks are possible
==1155442==
==1155442== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
==1155442==
==1155442== 1 errors in context 1 of 1:
==1155442== Syscall param write(buf) points to uninitialised byte(s)
==1155442== at 0x4989887: write (write.c:26)
==1155442== by 0x48FFEEC: _IO_file_write@@GLIBC_2.2.5 (fileops.c:1180)
==1155442== by 0x49019E0: new_do_write (fileops.c:448)
==1155442== by 0x49019E0: _IO_new_do_write (fileops.c:425)
==1155442== by 0x49019E0: _IO_do_write@@GLIBC_2.2.5 (fileops.c:422)
==1155442== by 0x4900FD7: _IO_file_close_it@@GLIBC_2.2.5 (fileops.c:135)
==1155442== by 0x48F3D8E: fclose@@GLIBC_2.2.5 (iofclose.c:53)
==1155442== by 0x10991E: saveToFile (in /home/mark/ESP32-Projects/esp32-launcher/sandbox/users-binary_save/testUsers)
==1155442== by 0x109D0E: main (in /home/mark/ESP32-Projects/esp32-launcher/sandbox/users-binary_save/testUsers)
==1155442== Address 0x4aa17f6 is 6 bytes inside a block of size 4,096 alloc'd
==1155442== at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==1155442== by 0x48F3BA3: _IO_file_doallocate (filedoalloc.c:101)
==1155442== by 0x4902CDF: _IO_doallocbuf (genops.c:347)
==1155442== by 0x4901F5F: _IO_file_overflow@@GLIBC_2.2.5 (fileops.c:744)
==1155442== by 0x49006D4: _IO_new_file_xsputn (fileops.c:1243)
==1155442== by 0x49006D4: _IO_file_xsputn@@GLIBC_2.2.5 (fileops.c:1196)
==1155442== by 0x48F4FD6: fwrite (iofwrite.c:39)
==1155442== by 0x1098A5: saveToFile (in /home/mark/ESP32-Projects/esp32-launcher/sandbox/users-binary_save/testUsers)
==1155442== by 0x109D0E: main (in /home/mark/ESP32-Projects/esp32-launcher/sandbox/users-binary_save/testUsers)
==1155442== Uninitialised value was created by a heap allocation
==1155442== at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==1155442== by 0x1095F6: findUser (in /home/mark/ESP32-Projects/esp32-launcher/sandbox/users-binary_save/testUsers)
==1155442== by 0x1094F4: createUser (in /home/mark/ESP32-Projects/esp32-launcher/sandbox/users-binary_save/testUsers)
==1155442== by 0x1093DE: initDB (in /home/mark/ESP32-Projects/esp32-launcher/sandbox/users-binary_save/testUsers)
==1155442== by 0x109AFF: main (in /home/mark/ESP32-Projects/esp32-launcher/sandbox/users-binary_save/testUsers)
users.h
#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>
#define MAX_USERS 3
#define MAX_USER_NAME_SIZE 20
#define PERMISSION_LAUNCH 4
#define PERMISSION_EDIT_USER 2
#define PERMISSION_EDIT_LAUNCHER 1
typedef struct {
char user_name[20];
char password[20];
int permissions;
int index;
} User;
typedef struct {
int code;
char *message;
char* payload;
} Error;
extern char* user_names[MAX_USERS][MAX_USER_NAME_SIZE];
Error create_error(int code, char *message);
Error createUser(char* user_name, char* password, int permissions);
User* findUser(char * user_name);
void printUsers();
Error initDB();
void deleteUsers();
Error saveToFile();
Error readFromFile();
users.c
#include <stdio.h>
#include <stdbool.h>
#include <stdint.h>
#include <string.h>
#include <stdlib.h>
#include "users.h"
/* An array of pointers to the User structs created */
static User* users_storage[MAX_USERS];
/* The number of users in the users_storage array */
int user_counter = 0;
/* File name for saving the users_storage to disk */
const char* USER_FILE_NAME = "users.dat";
/*
A struct to return error messages to the UI as needed for errors that come up.
There are no different error codes, just 0 for success and -1 for an error. The UI
Would check the error code and then display the error message.
*/
Error create_error(int code, char *message) {
Error err;
err.code = code;
err.message = message;
err.payload = "Nothing to see here.";
return err;
}
/*
Null out the users_storage and add the "admin" user to the users_storage
*/
Error initDB() {
Error result = create_error(0, "Success");
for (int i = 0; i < MAX_USERS; i++) {
if (users_storage[i] != NULL) {
free(users_storage[i]);
}
users_storage[i] = NULL;
}
int permissions = PERMISSION_LAUNCH + PERMISSION_EDIT_USER + PERMISSION_EDIT_LAUNCHER;
result = createUser("admin", "password", permissions);
return result;
}
/*
Create a new User.
- Checks for invalid permissions; it must be between 1 and 7
- Checks for duplicate users using the user name
- Creates a new user and adds it to the users_storage
- Increments the user_counter
- If the users_storage is full (ie there are MAX_USERS in the users_storage), then an error is returned
with a message for the UI that a user must be deleted before a new one can be created.
*/
Error createUser(char* user_name, char* password, int permissions) {
int user_permissions = 0;
Error result = create_error(0, "Success");
if (permissions < 0 || permissions > 7) {
result.code = -1;
result.message = "Invalid permissions. Permissions must be between 0 and 7. Permissions set to 1.";
user_permissions = 1;
}
else {
user_permissions = permissions;
}
User* duplicate = findUser(user_name);
if (duplicate != NULL) {
result.message = "Duplicate user";
result.code = -1;
return result;
}
User* user = findUser(NULL);
if (user != NULL) {
strcpy(user->user_name, user_name);
strcpy(user->password, password);
user->permissions = user_permissions;
user_counter++;
return result;
}
else {
result.message = "Out of user memory. Delete a user before creating another one.";
result.code = -1;
return result;
}
}
/*
Finds a user by user name.
- Returns NULL if the user does not exist
- Returns the User struct if the user is found
*/
User* findUser(char* user_name) {
for (int i = 0; i < MAX_USERS; i++) {
User * user = users_storage[i];
if (user == NULL && user_name == NULL) {
// found and empty user slot
User * user = malloc(sizeof(User));
user->index = i;
users_storage[i] = user;
return user;
}
if (user != NULL && user_name != NULL) {
if (strcmp(user->user_name, user_name) == 0) {
return user;
}
}
}
return NULL;
}
/*
Convenience function to print out the contents of the users_storage in a human
readable format for debugging.
*/
void printUsers() {
for (int i = 0; i < MAX_USERS; i++) {
User* user = users_storage[i];
if (user != NULL) {
printf("%i, %s, %s, %i, %i\n", i, user->user_name, user->password, user->permissions, user->index);
}
else {
printf("%i, NULL\n", i);
}
}
}
/*
Used by readFromFile to clear the users_storage.
*/
void deleteUsers() {
for (int i=0; i<MAX_USERS; i++) {
if (users_storage[i] != NULL) {
User* user = users_storage[i];
printf("deleteUsers user_name=%s\n", user->user_name);
free(user);
users_storage[i] = NULL;
}
}
}
/*
Write the users_storage to a binary file
*/
Error saveToFile() {
Error result = create_error(0, "Success");
FILE *file = fopen(USER_FILE_NAME, "wb");
if (file == NULL) {
result.message = "Error opening file";
result.code = -1;
return result;
}
size_t element_count = 0;
for (int i = 0; i < MAX_USERS; i++) {
if (users_storage[i] != NULL) {
User* user = users_storage[i];
//printf("user_name=%s, password=%s, permissions=%i, index=%i\n", user->user_name, user->password, user->permissions, user->index);
fwrite(user, sizeof(User), 1, file);
if (ferror(file)) {
perror("Error writing to file");
result.message = "Error writing to file";
result.code = -1;
fclose(file);
return result;
}
element_count++;
}
}
fclose(file);
result.payload = "User data saved in a file";
return result;
}
/*
Read the users_storage from a binary file
*/
Error readFromFile() {
Error result = create_error(0, "Success");
FILE *file = fopen(USER_FILE_NAME, "rb");
if (file == NULL) {
result.message = "Error opening file";
result.code = -1;
return result;
}
User user;
deleteUsers();
size_t read_size;
int i = 0;
int users_created = 0;
while(1) {
read_size = fread(&user, sizeof(User), 1, file);
if (read_size == 0) {
break;
}
else {
createUser(user.user_name, user.password, user.permissions);
i++;
users_created++;
}
}
fclose(file);
result.payload = "User data read from file.";
return result;
}
testUsers.c
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#include "users.h"
int main() {
printf("InitDB: ");
Error result = initDB();
printf("%i, %s\n",result.code, result.message);
printUsers();
//create a user with bad permissions
printf("\ncreate a user ron with bad permissions (9): ");
result = createUser("ron", "password", 9);
printf("%i, %s\n",result.code, result.message);
printUsers();
// create a user
printf("\ncreate john: ");
result = createUser("john", "pw0", 4);
printf("%i, %s\n",result.code, result.message);
printUsers();
// create the same user again
printf("\ncreate john: ");
result = createUser("john", "pw0", 4);
printf("%i, %s\n",result.code, result.message);
printUsers();
// create another user when out of users
// Need to set MAX_USERS to 4 to trip this error condition
printf("\ncreate mark: ");
result = createUser("mark", "pw0", 4);
printf("%i, %s\n",result.code, result.message);
printUsers();
printf("\nSaving the file: \n");
result = saveToFile();
printf("%i, %s, %s\n",result.code, result.message, result.payload);
printUsers();
printf("\nReading the file: \n");
result = readFromFile();
printf("%i, %s, %s\n",result.code, result.message, result.payload);
printUsers();
printf("\nDeleting all the users\n");
deleteUsers();
return(0);
}
testRead.c
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#include "users.h"
int main() {
printf("\nReading the file: \n");
Error result = readFromFile();
printf("%i, %s, %s\n",result.code, result.message, result.payload);
printUsers();
printf("\nRemove all the users from memory\n");
deleteUsers();
return(0);
}
Makefile
usermake: users.c testUsers.c
gcc -o -g -O0 -o testUsers users.c testUsers.c -I .
gcc -o -g -O0 -o testRead users.c testRead.c -I .
When you use malloc to allocate space for a User object, the allocated bytes are uninitialized. When you later write to the username and password fields using strcpy, this doesn't write all elements of the array. This leave any remaining array elements uninitialized.
When you then write the struct to disk, you're writing the entire struct, including the uninitialized elements of the username and password arrays. This is what valgring is catching.
What you can do here is use calloc instead of malloc to allocate the space. This will initialize all bytes of the returned memory to 0 and prevent any reading of uninitialized bytes.
Unrelated to this, it doesn't make sense for the findUser function to be creating a new User object. That should be the responsibility of the createUser function. The findUser function should only find a user.