I apologise in advance for what will be a bit of a code dump, I've trimmed as much unimportant code as possible:
// Global vars / mutex stuff
extern char **environ;
pthread_mutex_t env_mutex = PTHREAD_MUTEX_INITIALIZER;
int
putenv_r(char *string)
{
int len;
int key_len = 0;
int i;
sigset_t block;
sigset_t old;
sigfillset(&block);
pthread_sigmask(SIG_BLOCK, &block, &old);
// This function is thread-safe
len = strlen(string);
for (int i=0; i < len; i++) {
if (string[i] == '=') {
key_len = i; // Thanks Klas for pointing this out.
break;
}
}
// Need a string like key=value
if (key_len == 0) {
errno = EINVAL; // putenv doesn't normally return this err code
return -1;
}
// We're moving into environ territory so start locking stuff up.
pthread_mutex_lock(&env_mutex);
for (i = 0; environ[i] != NULL; i++) {
if (strncmp(string, environ[i], key_len) == 0) {
// Pointer assignment, so if string changes so does the env.
// This behaviour is POSIX conformant, instead of making a copy.
environ[i] = string;
pthread_mutex_unlock(&env_mutex);
return(0);
}
}
// If we get here, the env var didn't already exist, so we add it.
// Note that malloc isn't async-signal safe. This is why we block signals.
environ[i] = malloc(sizeof(char *));
environ[i] = string;
environ[i+1] = NULL;
// This ^ is possibly incorrect, do I need to grow environ some how?
pthread_mutex_unlock(&env_mutex);
pthread_sigmask(SIG_SETMASK, &old, NULL);
return(0);
}
As the title says, I'm trying to code a thread safe, async-signal safe reentrant version of putenv
. The code works in that it sets the environment variable like putenv
would, but I do have a few concerns:
- My method for making it async-signal safe feels a bit ham-handed, just blocking all signals (except SIGKILL/SIGSTOP of course). Or is this the most appropriate way to go about it.
- Is the location of my signal blocking too conservative? I know
strlen
isn't guaranteed to be async-signal safe, meaning that my signal blocking has to occur beforehand, but perhaps I'm mistaken. - I'm fairly sure that it is thread safe, considering that all the functions are thread-safe and that I lock interactions with
environ
, but I'd love to be proven otherwise. - I'm really not too sure about whether it's reentrant or not. While not guaranteed, I imagine that if I tick the other two boxes it'll most likely be reentrant?
I found another solution to this question here, in which they just set up the appropriate signal blocking and mutex locking (sick rhymes) and then call putenv
normally. Is this valid? If so, it's obviously far simpler than my approach.
Sorry about the large block of code, I hope I've established a MCVE. I'm missing a bit of error checking in my code for brevity's sake. Thanks!
Here is the rest of the code, including a main, if you wish to test the code yourself:
#include <string.h>
#include <errno.h>
#include <pthread.h>
#include <stdlib.h>
#include <stdio.h>
#include <signal.h>
// Prototypes
static void thread_init(void);
int putenv_r(char *string);
int
main(int argc, char *argv[]) {
int ret = putenv_r("mykey=myval");
printf("%d: mykey = %s\n", ret, getenv("mykey"));
return 0;
}
This code is a problem:
It creates a
char *
on the heap, assigns the address of thatchar *
toenviron[i]
, then overwrites that value with the address contained instring
. That's not going to work. It doesn't guarantee thatenviron
is NULL-terminated afterwards.Because
char **environ
is a pointer to an array of pointers. The last pointer in the array isNULL
- that's how code can tell it's reached the end of the list of environment variables.Something like this should work better:
Note that there's no error checking, and it assumes the original
environ
array was obtained via a call tomalloc()
or similar. If that assumption is wrong, the behavior is undefined.