Hi,
On Sun, May 08, 2016 at 11:41:17AM +0200, Maxime de Roucy wrote:
> If -f argument is a directory add all the files (and only files) it
> containes to the config files list.
I think this is a nice addition, it completes well the ability to load
an arbitrary file list.
However I cannot merge it as is, there is a huge buffer overflow here :
> + char filename[MAX_CLI_DIRFILE_NAME];
> + struct dirent *dir_entry = dir_entries[dir_entries_it];
> +
> + strcpy(filename, wl->s);
> + strcat(filename, "/");
> + strcat(filename, dir_entry->d_name);
You'd rather use snprintf("%s/%s") and check the result.
As a hint, you should always consider that strcpy() with a variable on
input is almost always a bug, and that strcat() is always a bug.
I have another comment here, please try to factor the error messages
using a goto, this block appears at least 3 times :
> +
> + if (stat(filename, &file_stat)) {
> + Alert("Cannot open configuration file %s :
> %s\n",
> + filename,
> + strerror(errno));
> + exit(1);
> + }
I think it would be easier to move all this patch into a dedicated function.
Here instead of calloc+strcpy, you can simply use strdup() :
> + wlt->s = calloc(strlen(filename) + 1,
> sizeof(*wlt->s));
> + if (!wlt->s) {
> + Alert("Cannot load configuration files
> %s : out of memory.\n",
> + filename);
> + exit(1);
> + }
> + strcpy(wlt->s, filename);
Otherwise it looks good.
Thanks,
Willy
