[ABRT PATCH] add environment variable whitelist to debuginfo install wrapper - closes 692
Jakub Filak
jfilak at redhat.com
Tue Sep 10 13:43:38 UTC 2013
It looks good to me except some coding style issues.
> + // However since we communicate through environment variables
> + // we have to keep a whitelist of variables to keep.
> + static const char *whitelist[] = {
> + "REPORT_CLIENT_SLAVE" // Check if the app is being run as a slave
> + };
> + int wlsize = sizeof(whitelist)/sizeof(char*);
I'd use a more accurate type. Maybe size_t or rather const size_t.
> + char *setlist[wlsize];
You can statically initialize the entire array to NULLs by assigning
NULL to the first element.
char *setlist[sizeof(whitelist)/sizeof(char*)] = { NULL };
> + char *p = NULL;
> + for (int i = 0; i < wlsize; i++) {
for (size_t i = 0; i < wlsize; ++i)
{
> + if ((p = getenv(whitelist[i])) != NULL)
{
> + setlist[i] = malloc(strlen(p)*sizeof(char*));
> + setlist[i] = strncpy(setlist[i], p, strlen(p));
How about:
setlist[i] = xstrdup(p);
> + }
> + else
> + setlist[i] = NULL;
And if the array is initialized to NULLs, you can drop the two above
lines.
> + }
> + // Now we can clear the environment
> clearenv();
> + // And once again set whitelisted variables
> + for (int i = 0; i < wlsize; i++)
{
> + if (setlist[i] != NULL)
{
How about:
xsetenv(whitelist[i], setlist[i]);
> + free(setlist[i]);
> + }
> + }
> #else
> /* Clear dangerous stuff from env */
> static const char forbid[] =
More information about the Crash-catcher
mailing list