Many similar posts exist on Stack Overflow, I have carefully reviewed such posts and have linked them to the end of this post. These existing posts have not been helpful, because they demonstrate segfaults resulting from passing uninitialized portions of memory into the char *dest argument of the strncpy() function. Another common theme in these posts are strong recommendations against using strncpy(), but the only alternative recommendation I have read is to use strlcpy() instead.
My program uses a statically allocated array of FS_Info structs to store information about file system entries. The idea is to output the names and sizes of the 10 largest files in a specified directory. As the array is of fixed size, when a filesystem entry is found that is larger than the smallest entry in the array, my program attempts to update the struct describing this smaller entry with values describing the new, larger one.
#include <stdio.h>
#include <stdlib.h>
#include <dirent.h>
#include <sys/stat.h>
#include <string.h>
#include <limits.h>
#include <unistd.h>
// size of array used to contain filesystem entries
const size_t fs_info_arr_size = 10;
// A struct to contain the name of a filesystem entry and its size in bytes.
typedef struct FS_Info
{
char name[PATH_MAX];
long long size;
} FS_Info;
// global pointer to FS_Info array
FS_Info *fs_info_arr[fs_info_arr_size];
// used to sort fs_entries array descending in terms of entry size
static int compare(const void *a, const void *b)
{
const struct FS_Info *entryA = (FS_Info *)a;
const struct FS_Info *entryB = (FS_Info *)b;
return (entryB->size - entryA->size) - (entryA->size - entryB->size);
}
/*
Iterates over an array of FS_Info structs and returns a pointer to the struct having the smallest size member
*/
FS_Info *get_smallest_entry(FS_Info **entries)
{
long long smallest = entries[0]->size;
FS_Info *target;
for (int i = 1; i < fs_info_arr_size * sizeof(FS_Info); i = sizeof(FS_Info))
{
if (entries[i]->size < smallest)
{
smallest = entries[i]->size;
target = entries[i];
}
}
return target;
}
/*
Add entires to the array. If the array is full, use the above function to find the
struct having the smallest file size, and if the current file size is larger, replace it.
*/
void update_fs_info_arr(char *path) // FIXME debug call stack shows that using strncpy here causes segfault
{
static int items_added = 0;
struct stat st;
if (stat(path, &st) == 0)
{
if (items_added < fs_info_arr_size) // if array capacity will not be exceeded
{
strncpy(fs_info_arr[items_added]->name, path, PATH_MAX);
// strlcpy(fs_info_arr[items_added]->name, path, sizeof(fs_info_arr[items_added]->name));
// strncpy(fs_info_arr[items_added]->name, path, sizeof(fs_info_arr[items_added]->name) / sizeof(fs_info_arr[items_added]->name[0]) - 1);
// fs_info_arr[items_added]->name[sizeof(fs_info_arr[items_added]->name) / sizeof(fs_info_arr[items_added]->name[0]) - 1] = 0;
fs_info_arr[items_added]->size = st.st_size;
items_added ;
}
else
// find entry having the smallest size and replace it with the current entry if it is larger
{
FS_Info *smallest = get_smallest_entry(fs_info_arr);
if (st.st_size > smallest->size)
{
strncpy(smallest->name, path, PATH_MAX);
// strlcpy(smallest->name, path, sizeof(smallest->name));
// strncpy(smallest->name, path, sizeof(smallest->name) / sizeof(smallest->name[0]) - 1);
// smallest->name[sizeof(smallest->name) / sizeof(smallest->name[0]) - 1] = 0;
smallest->size = st.st_size;
}
}
}
else
{
printf("Error getting stat for entry %s: %d\n", path, stat(path, &st));
}
}
void walk(const char *currDir)
{
DIR *dir = opendir(currDir);
struct dirent *entry;
if (dir == NULL)
{
printf("%s could not be opened", currDir);
return;
}
while ((entry = readdir(dir)) != NULL)
{
// if directory is current dir or parent dir
if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0)
{
continue;
}
char path_to_entry[PATH_MAX];
snprintf(path_to_entry, sizeof(path_to_entry), "%s/%s", currDir, entry->d_name);
//snprintf(path_to_entry, sizeof(path_to_entry) - 1, "%s/%s", currDir, entry->d_name);
//path_to_entry[sizeof(path_to_entry) - 1] = '\0';
update_fs_info_arr(path_to_entry);
if (entry->d_type == DT_DIR)
{
walk(path_to_entry);
}
}
closedir(dir);
}
int main(int argc, char *argv[])
{
char target_dir[PATH_MAX];
strncpy(target_dir, argv[1], PATH_MAX);
printf("Finding the %zu largest files in: %s\n", fs_info_arr_size, target_dir);
// recursively visit all entries in the specified directory
walk(target_dir);
// sort the entries descending by file size
qsort(fs_info_arr, fs_info_arr_size, sizeof(*fs_info_arr), compare);
// output ten largest files found
for (int i = 0; i < fs_info_arr_size; i )
{
printf("%s\t%lld\n", fs_info_arr[i]->name, fs_info_arr[i]->size);
}
return EXIT_SUCCESS;
}
The posts linked below fail to initialize memory being copied into by strncpy() or involve an incorrect size_t num argument as specified here. In the case of the later, I attempted changing my code to match the pattern described in the post linked first below, but this had no effect.
strncpy leading to segmentation fault
Segmentation fault when calling strcpy
Segmentation fault: 11 (caused by strncpy())
Why do I get a segmentation fault when using strncpy?
Segmentation Fault when using strncpy in c
Edit:
I've made fixes as suggested in the answer section and realize why my program logic was flawed, but this program still results in a bad access:
#include <stdio.h>
#include <stdlib.h>
#include <dirent.h>
#include <sys/stat.h>
#include <string.h>
#include <limits.h>
#include <unistd.h>
// size of array used to contain filesystem entries
const size_t fs_info_arr_size = 10;
/*
A struct to contain the name of a filesystem entry and its size in bytes.
An array of this type will be used to catalog all filesystem entries for
the directory specified as command line argument.
*/
typedef struct FS_Info
{
char name[PATH_MAX];
long long size;
} FS_Info;
// global pointer to FS_Info array
FS_Info fs_info_arr[fs_info_arr_size];
// used to sort fs_entries array descending in terms of entry size
static int compare(const void *a, const void *b)
{
const struct FS_Info *entryA = (FS_Info *)a;
const struct FS_Info *entryB = (FS_Info *)b;
return entryB->size - entryA->size;
}
/*
Iterates over an array of FS_Info structs and returns a pointer to the struct
having the smallest size member.
*/
FS_Info *get_smallest_entry(FS_Info *entries)
{
long long smallest = entries[0].size;
FS_Info *target;
for (int i = 1; i < fs_info_arr_size; i )
{
if (entries[i].size < smallest)
{
smallest = entries[i].size;
target = &entries[i];
}
}
return target;
}
/*
Add entires to the array. If the array is full, use the above function to find the
struct having the smallest file size, and if the current file size is larger, replace it.
*/
void update_fs_info_arr(char *path) // FIXME debug call stack shows that using strncpy here causes segfault
{
static int items_added = 0;
struct stat st;
if (stat(path, &st) == 0)
{
if (items_added < fs_info_arr_size) // if array capacity will not be exceeded
{
strncpy(fs_info_arr[items_added].name, path, PATH_MAX);
fs_info_arr[items_added].size = st.st_size;
items_added ;
}
else
// find entry having the smallest size and replace it with the current entry if it is larger
{
FS_Info *smallest = get_smallest_entry(fs_info_arr);
if (st.st_size > smallest->size)
{
strncpy(smallest->name, path, PATH_MAX);
smallest->size = st.st_size;
}
}
}
else
{
printf("Error getting stat for entry %s: %d\n", path, stat(path, &st));
}
}
void walk(const char *currDir)
{
DIR *dir = opendir(currDir);
struct dirent *entry;
if (dir == NULL)
{
printf("%s could not be opened", currDir);
return;
}
while ((entry = readdir(dir)) != NULL)
{
// if directory is current dir or parent dir
if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0)
{
continue;
}
char path_to_entry[PATH_MAX];
snprintf(path_to_entry, sizeof(path_to_entry), "%s/%s", currDir, entry->d_name);
update_fs_info_arr(path_to_entry);
if (entry->d_type == DT_DIR)
{
walk(path_to_entry);
}
}
closedir(dir);
}
int main(int argc, char *argv[])
{
// a char array to hold a filesystem path
char target_dir[PATH_MAX];
strncpy(target_dir, argv[1], PATH_MAX);
printf("Finding the %zu largest files in: %s\n", fs_info_arr_size, target_dir);
// recursively visit all entries in the specified directory
walk(target_dir);
// sort the entries descending by file size
qsort(fs_info_arr, fs_info_arr_size, sizeof(*fs_info_arr), compare);
// output ten largest files found
for (int i = 0; i < fs_info_arr_size; i )
{
printf("%s\t%lld\n", fs_info_arr[i].name, fs_info_arr[i].size);
}
return EXIT_SUCCESS;
}
CodePudding user response：
OP: "My program uses a statically allocated array of FS_Info structs to store information about file system entries."
Code:
FS_Info *fs_info_arr[fs_info_arr_size];
You are allocating pointers, but never allocate space to store what the pointers might point at...
Try
FS_Info fs_info_arr[fs_info_arr_size];
and make the correct adaptations to the code to use the array of structs (not the array of pointers that are all NULL.)
And...
static int compare(const void *a, const void *b)
{
const struct FS_Info *entryA = (FS_Info *)a;
const struct FS_Info *entryB = (FS_Info *)b;
return (entryB->size - entryA->size) - (entryA->size - entryB->size);
}
is needlessly complex. For descending ordering simply use
return entryB->size - entryA->size;
(and you can apologise to strncpy() for the inappropriate slurring of its reputation.)