2016-12-21 23 views
3

は、私は可能な限り重要でないコードをトリミングしてきた、コードダンプのビットがどうなるかを事前に謝罪:スレッドセーフ、再入可能、非同期シグナル安全putenvを

// 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); 
} 

タイトルが言うように私はputenvのスレッドセーフな非同期シグナル安全なリエントラントバージョンをコーディングしようとしています。コードは、それがputenvのように環境変数を設定していることでしょうで動作しますが、私はいくつかの懸念を持っている:

  1. 非同期シグナル安全にそれを作るための私の方法は、単にすべての信号(除く、ブロッキング、ビットハム利きを感じていますSIGKILL/SIGSTOPはもちろんです)。それともこれが最も適切な方法でしょうか。
  2. 私の信号の位置があまりにも控えめなのですか?私はstrlenが非同期信号の安全であることを保証していないことを知っています。つまり、私の信号遮断があらかじめ発生している必要がありますが、間違いです。
  3. すべての関数がスレッドセーフであり、私がenvironとのやりとりをロックしていることを考えると、スレッドセーフであることはかなり確信していますが、そうでなければ実証されたいと思います。
  4. 私は本当にそれがリエントラントかどうかについてもあまり確信していません。保証されているわけではありませんが、私が他の2つのボックスにチェックを入れると、再入可能性が高いでしょう。

彼らは単に適切な信号ブロッキングと相互排他ロック(病気韻)を設定した後、通常putenvを呼び出したこの質問hereに対する別の解決策を見つけました。これは有効ですか?もしそうなら、明らかに私のアプローチよりもはるかに単純です。

申し訳ありませんコードの大部分については、MCVEを設定していただきたいと思います。私は、簡潔さのために私のコードでエラーチェックを少し欠いています。ありがとう!あなたは、コードを自分でテストしたい場合はここで


は、メインを含め、残りのコードです:

#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; 
} 
+0

オフトピックですが、Cはゼロベースの配列を持っているので、 'key_len = i;' key_len = i-1; ' –

+0

@KlasLindbäckにします。 'strncmp' * key = value *の* key *部分です。ループブレイクが '= '記号の位置にあるときに' 'i' 'となるので、インデックスから1を引く必要があります。 –

+1

'strncmp'は最後の文字のインデックスではなく、文字数を求めます。 'foo = bar'ならば' i = 3'と 'key_len = 2'となり、' strncmp'は最初の2文字だけを比較します。 –

答えて

2

このコードは問題です:

// 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; 

それが作成されますchar *をヒープ上に配置し、そのアドレスchar *environ[i]に割り当て、その値をstringに含まれるアドレスで上書きします。それはうまくいかないでしょう。その後、environがNULLで終了することを保証するものではありません。

char **environ is a pointer to an array of pointersです。配列の最後のポインタはNULLです。これは、コードが環境変数のリストの最後に到達したことを示す方法です。エラーチェックがないことを

unsigned int envCount; 

for (envCount = 0; environ[ envCount ]; envCount++) 
{ 
    /* empty loop */; 
} 

/* since environ[ envCount ] is NULL, the environ array 
    of pointers has envCount + 1 elements in it */ 
envCount++; 

/* grow the environ array by one pointer */ 
char ** newEnviron = realloc(environ, (envCount + 1) * sizeof(char *)); 

/* add the new envval */ 
newEnviron[ envCount - 1 ] = newEnvval; 

/* NULL-terminate the array of pointers */ 
newEnviron[ envCount ] = NULL; 

environ = newEnviron; 

注意し、それが元environ配列はmalloc()または類似の呼び出しを介して取得した前提としています。このような

何かがよりよく動作するはずです。その仮定が間違っている場合、その動作は未定義です。

+0

'execve'を呼び出した後自然に提供される' environ'を使っています。 'malloc'を必要とするかどうかは分かりません。しかし、これを指摘してくれてありがとう、私は環境に新しい要素を加える方法が不十分だったという気持ちを持っていました。 –

+0

私はこれらの提案を反映するために、 'newEnviron'のステップをバイパスし、' environ'を直接使用して質問を更新します。私はコードにはまだ行く方法があると思う。 –

+0

@DanielPorteous 'environ'自体で直接作業する場合、コードはスレッドセーフではありません。厳密に言えば、ポインタ値の更新は必ずしもアトミックな操作であるとは限らないため、 'environ = newEnviron;でさえ必ずしもスレッドセーフではありません。 –

関連する問題