c言語 動的確保 関数化

Question

質問

関数を使って動的確保をしたいのですが、どちらの書き方の方がいいですか?
また文法的に間違っていたら教えてくださいm(__)m

該当のソースコード

//include

int *sec1(int n){
	int *p = (int*)malloc(sizeof(int)*n);
	return p;
}

void sec2(int n,int *p2){
	p2 = (int*)malloc(sizeof(int)*n);
}


///////////////////(追記)//////////////////////////////////////
void Free1(int **p1){
        free(*p1);
}

void Free2(int *p2){
        free(p2);
}
//////////////////////////////////////////////////////////////

int main()
{
	int n = 1;
	int *p1,*p2;

	p1 = sec1(n);
	sec2(n,p2);

	Free1(&p1);
	Free2(p2);
	return 0;
}

Answer 1

本例では関数が小さすぎます。

現実的な関数でないので、一般的な議論になりません。ケースバイケースとしか回答できません。ちなみに、この程度の大きさの関数ならば、malloc、freeをそのまま使います。

Comments

[HogeAnimalLover]

強いていうと、「free直後にポインタ変数をNULL埋めする」というfreeラッパ関数ならば価値はあると思います。若干ですが。

Answer 2

sec1の書き方のほうが可読性・再利用性の観点から好ましいと思います。

質問にある2つの関数(*sec1,sec2)の違いは、「戻り値の有無」になります。
そして、過去にあった同様の質問をみると、

引数を与えたら戻り値が返ってくる、というプログラミングモデルは人間にわかりやすい

参照を埋め込むよりは戻り値で済むなら戻り値で返します。

そういう依存の少ないメソッドを組み合わせてシステムを作ると
複雑なシステムでも後で変更しやすくなります。

などとコメントがされています。詳しくは以下の記事をご覧ください。

戻り値あり、戻り値なしの使い分けについて

ご参考になれば幸いです。

Comments

[cateye]

同意見です。
・パラメータが少ない。
・復帰値で判定が出来る

など

Answer 3

void sec2(int n,int *p2){
    p2 = (int*)malloc(sizeof(int)*n);
}

これ↑マチガイ。正しくは↓

void sec2(int n,int **p2) {
    *p2 = (int*)malloc(sizeof(int)*n);
}

※呼び出し側も修正してな。

で、どっちがいいかは場合による。

Answer 4

sec2の間違いは他回答通りです。
sec1とsec2、またFree1とFree2のどちらが良いかということについては、どれもケースバイケースだし、もっと言えば不要な関数です。
元々１ステップで書ける処理なので関数化することに意味がないからです。
freeの関数化については、以下のようにNULLではないとき（動的確保されているとき）のみに解放するような処理が望ましいので、関数化する意義はあります。

void Free(int *p){
    if(p){
        free(p);
    }
}

Comments

[HogeAnimalLover]

これは不要では？「free(NULL)」は「何もしないこと」と定められているはずです。ポインタをNULL埋めすることのほうが有益です。

[ttyp03]

今まで特に意識してなく、過去にそのような書き方をしてたのを見てたので、そのほうが安全だと思いこんでいたようです。
ひとつ勉強になりました。
ただし、注意してくださるなら"はず"という曖昧な情報ではなく、その根拠まで示してください。
すべてのコンパイラで保証されているならいいのですが、そうではないコンパイラがある可能性があるならまったく不要な処理とも言えないと思います。

Answer 5

void sec2(int n,int *p2){

p2 = (int*)malloc(sizeof(int)*n);

}

これはだめですね

void sec2(int n,int **p2){
p2 = (int)malloc(sizeof(int)*n);
}

Comments

[fumi6328]

そうなんですか、ではもう一つだけ聞きたいのですが、freeを別関数内で行うとしたら
Free1が正しくFree2は間違っているということですか?(追記したところです)

[y_waiwai]

なぜsec2がだめかというと、アドレスという値を返さなくてはならないのに、
p2 = なんやかや
では、呼び出し元に値を返せないから、です
そこで、アドレスのポインタを引数にすることで、ポインタの指し先にアドレスを設定するようにします

で、Free1は間違ってはいない、けど、ここはアドレスのポインタにする必要はないです
freeするのには、アドレスを渡せばいいだけで、呼び出し元に戻す必要はないからです

#free1をfree2と間違って記述してたので訂正

[fumi6328]

ありがとうございました！