12

呼び出し元にメモリを割り当てる関数があるとします。

int func(void **mem1, void **mem2) {
    *mem1 = malloc(SIZE);
    if (!*mem1) return 1;

    *mem2 = malloc(SIZE);
    if (!*mem2) {
        /* ... */
        return 1;
    }

    return 0;
}

2 番目の malloc() が失敗した場合に備えて、割り当てられたメモリを free() する最善の方法について、フィードバックをお寄せください。より多くのエラー終了ポイントとより多くのメモリが割り当てられた、より複雑な状況を想像できます。

4

9 に答える 9

26

私は人々がそれらを使うことを嫌うことを知っています、しかしこれはgotoCのaにとって完璧な状況です。

int func( void** mem1, void** mem2 )
{
    int retval = 0;
    *mem1 = malloc(SIZE);
    if (!*mem1) {
        retval = 1;
        goto err;
    }

    *mem2 = malloc(SIZE);
    if (!*mem2) {
        retval = 1;
        goto err;
    }
// ...     
    goto out;
// ...
err:
    if( *mem1 ) free( *mem1 );
    if( *mem2 ) free( *mem2 );
out:
    return retval;
}
于 2009-02-20T17:05:56.080 に答える
7

私の意見では、これは後藤が適切な場所です。以前は反後藤の教義に従っていましたが、{...} while（0）;を行うことが指摘されたときに変更しました。同じコードにコンパイルされますが、読みやすくはありません。逆行しない、最小限に抑える、エラー状態にのみ使用するなど、いくつかの基本的なルールに従ってください...

int func(void **mem1, void **mem2)
{
    *mem1 = NULL;
    *mem2 = NULL;

    *mem1 = malloc(SIZE);
    if(!*mem1)
        goto err;

    *mem2 = malloc(SIZE);
    if(!*mem2)
        goto err;

    return 0;
err:
    if(*mem1)
        free(*mem1);
    if(*mem2)
        free(*mem2);

    *mem1 = *mem2 = NULL;

    return 1;
}
于 2009-02-20T17:08:46.230 に答える
5

gotoこれは少し物議をかもしますが、Linuxカーネルで使用されるアプローチは実際にはこの状況でかなりうまく機能すると思います。

int get_item(item_t* item)
{
  void *mem1, *mem2;
  int ret=-ENOMEM;
  /* allocate memory */
  mem1=malloc(...);
  if(mem1==NULL) goto mem1_failed;

  mem2=malloc(...);
  if(mem2==NULL) goto mem2_failed;

  /* take a lock */
  if(!mutex_lock_interruptible(...)) { /* failed */
    ret=-EINTR;
    goto lock_failed;
  }

  /* now, do the useful work */
  do_stuff_to_acquire_item(item);
  ret=0;

  /* cleanup */
  mutex_unlock(...);

lock_failed:
  free(mem2);

mem2_failed:
  free(mem1);

mem1_failed:
  return ret;
}
于 2009-02-20T17:12:23.927 に答える
2

これは読みやすい代替手段です。

int func(void **mem1, void **mem2) {
  *mem1 = malloc(SIZE);
  *mem2 = malloc(SIZE);
  if (!*mem1 || !*mem2) {
    free(*mem2);
    free(*mem1);
    return 1;
  }
  return 0;
}
于 2009-05-17T06:33:25.873 に答える
1

呼び出し元は、障害が発生する前に正しく割り当てられたメモリブロックに対して何か便利なことをしますか？そうでない場合は、呼び出し先が割り当て解除を処理する必要があります。

クリーンアップを効率的に行う1つの可能性はdo..while(0)、を使用することです。これによりbreak、例をreturn次のようにできます。

int func(void **mem1, void **mem2)
{
    *mem1 = NULL;
    *mem2 = NULL;

    do
    {
        *mem1 = malloc(SIZE);
        if(!*mem1) break;

        *mem2 = malloc(SIZE);
        if(!*mem2) break;

        return 0;
    } while(0);

    // free is NULL-safe
    free(*mem1);
    free(*mem2);

    return 1;
}

多くの割り当てを行う場合は、freeAll()関数を使用してここでもクリーンアップを実行することをお勧めします。

于 2009-02-20T16:59:50.980 に答える
0

私自身の傾向は、NULL以外のすべてのポインターを解放する可変引数関数を作成することです。次に、呼び出し元はエラーケースを処理できます。

void *mem1 = NULL;
void *mem2 = NULL;

if (func(&mem1, &mem2)) {
    freeAll(2, mem1, mem2);
    return 1;
}
于 2009-02-20T16:53:55.017 に答える
0

上記の goto ステートメントが何らかの理由で恐ろしい場合は、いつでも次のようなことができます。

int func(void **mem1, void **mem2)
{
    *mem1 = malloc(SIZE);
    if (!*mem1) return 1;

    *mem2 = malloc(SIZE);
    if (!*mem2) {
        /* Insert free statement here */
        free(*mem1);
        return 1;
    }

    return 0;
}

私はこの方法をかなり定期的に使用していますが、何が起こっているのかが非常に明確な場合にのみ使用しています。

于 2009-05-17T06:12:32.460 に答える
-2

goto ステートメントに関するすべての推奨事項に、私は少し恐怖を感じています。

goto を使用するとコードが混乱し、プログラマーのエラーが発生しやすくなることがわかりました。私の好みは、最も極端な状況を除いて、その使用を完全に避けることです. 私はほとんどそれを使用することはありません。学問的な完璧主義のせいではなく、1年以上経つと、私が提案する代替案よりも全体的な論理を思い出すのが常に難しいように見えるからです.

ものを忘れる可能性を最小限に抑えるために物事をリファクタリングするのが大好きな人 (ポインターをクリアするなど) として、最初にいくつかの関数を追加します。同じプログラムでこれらをかなり再利用する可能性が高いと思います。関数 imalloc() は、間接ポインターを使用して malloc 操作を行います。ifree() はこれを元に戻します。cifree() は条件付きでメモリを解放します。

それを踏まえて、コードの私のバージョン (デモンストレーションのために 3 番目の引数を使用) は次のようになります。

// indirect pointer malloc
int imalloc(void **mem, size_t size)
{
   return (*mem = malloc(size));
}

// indirect pointer free
void ifree(void **mem)
{
   if(*mem)
   {
     free(*mem);
     *mem = NULL;
   }
}

// conditional indirect pointer free
void cifree(int cond, void **mem)
{
  if(!cond)
  {
    ifree(mem);
  }
}

int func(void **mem1, void **mem2, void **mem3)
{
   int result = FALSE;
   *mem1 = NULL;
   *mem2 = NULL;
   *mem3 = NULL;

   if(imalloc(mem1, SIZE))
   {
     if(imalloc(mem2, SIZE))
     {
       if(imalloc(mem3, SIZE))
       {
         result = TRUE;
       }            
       cifree(result, mem2);
     }
     cifree(result, mem1);
   }
  return result;
}

最後に、関数からの戻り値を 1 つだけにすることを好みます。間に飛び出すのは速いです（そして私の意見では、ちょっと汚いです）。しかし、もっと重要なのは、関連するクリーンアップ コードを意図せずに簡単にバイパスできるようにすることです。

于 2009-05-17T04:31:57.617 に答える