sizeof()のキャストについてのアンチパターン

Question

qiitaの記事を読んでC言語のプログラミングでのアンチパターンを学習しているのですが、

void func() {
  int loop = 0;
  struct aaa *aaa = NULL;
  aaa = (struct aaa *) malloc(sizeof(struct aaa *));
  if( NULL == aaa ) {
    return RET_NG;
  }
  // 何か処理
  free(aaa);
  return RET_OK;
}

サイトの解説によると、malloc時に指定したサイズに構造体の型ではなくポインタ型を指定すると、intサイズ分しかメモリを確保できないので
意図した動作にならないとあります。
なぜ、

aaa = malloc(sizeof(struct aaa)); // もしくはsizeof(*aaa)

とするのがベターなのでしょうか。

Answer 1

ベターとかじゃなくて単純にバグです

printf("sizeof(struct aaa*)=%d\n", sizeof(struct aaa*));
printf("sizeof(struct aaa) =%d\n", sizeof(struct aaa));

を比べてください

int分というかポインタサイズしか取得できません。

Comments

[imamoto_browser]

わかれば単純でした。結局割り当て方、間違ってるだけなんですね。

Answer 2

ちょっと元記事にも編集リクエスト投げましたが、こっちにも。

構造体を動的に確保する場合、普通はmalloc/calloc関数を利用すると思います。

ここで仮に構造体Hogeがあったとして

typedef struct {
  int value;
} Hoge;

要素数1だけ動的確保する場合

//                   構造体の大きさ * 要素数
Hoge* hoge_p = malloc(sizeof(Hoge) * 1);

のようにします(C89/C++ではできませんが、C99以降のCではvoid*型の値は他のポインタ型へ暗黙変換されるので、mallocの戻り値をキャストする必要はありません。)

ここでこのコードは次のコードと等価です。

Hoge* hoge_p = malloc(sizeof(Hoge));

でここまで書くと、「ふっふ～ん、わかった、mallocに渡すsizeofにはポインタじゃない型を書けばいいんでしょ？」という誤解をするかもしれません。そこで次のコードを見てください。

#include <stdio.h>
#include <stdlib.h>
typedef struct {
  int value;
} Hoge;

int main(void)
{
    const size_t col_size = 4;
    const size_t row_size = 3;
    Hoge** hoge_determinant = malloc(sizeof(Hoge*) * row_size);

    // init
    Hoge* tmp = calloc(sizeof(Hoge), col_size * row_size);
    for(size_t i = 0; i < row_size; ++i) hoge_determinant[i] = &tmp[i * row_size];

    //print
    for(size_t i = 0; i < row_size; ++i){
        putchar('|');
        for(size_t j = 0; j < col_size; ++j){
            if(j) putchar(' ');
            printf("%d", hoge_determinant[i][j].value);
        }
        puts("|");
    }

    return 0;
}

https://wandbox.org/permlink/esJmmwvPmD99qpbF

malloc(sizeof(Hoge*) * row_size)とあります。あれ、ちがうの？

ここでsizeof(Hoge*)としたのは、要素数row_sizeだけHoge*型の動的確保するためです。

すなわち、渡すべきは要素型の型の大きさです。

Answer 3

単純に「プログラムは意図した通りには動かない。書いた通りに動く」という格言通りなだけですね。

Answer 4

まず、ポインタのサイズと構造体のサイズが違うことは分かりますか？
構造体とポインタの名前が同じというのも誤解を招きますね。struct AAA *aaa= NULL;の方がまだ分かりやすいかな？
で、たとえば構図体のサイズが50バイトあった場合に、そのポインタは4バイト（32ビット環境の場合）になります。ようは、sizeof(AAA)とsizeof(aaa)は、各々50と4になる訳です。なので、4バイトの領域をはみ出して何か操作を行えば、確保した領域外にアクセスすることになります。場合によっては何も起こらないかもしれませんが、通常はメモリを破壊することになりますd＾＾・・・したがって、動いたり動かなかったり・・・厄介なバグになります。

＃記事の記述にも問題があります。”ポインタ型を指定した場合、intサイズ分しかメモリを確保できません”はポインタとintが同じ環境でしか意味を持ちません。正しくは、”ポインタのサイズ分”と言うべきだと思います。

Comments

[yumetodo]

これはひどい、元記事に編集リクエスト投げてきました

[cateye]

初心者＋向けみたいなページなので、出来れば初心者にも理解しやすい形で提供してもらいたいです。

[imamoto_browser]

>cateyaさん
回答ありがとうございます。構造体とポインタの名前は分けるべきというわけですね。
あとおっしゃることを言い換えると、mallocで構造体のメモリサイズを確保する際は、ポインタ型にキャストすべきではないということですね。
動いたり動かなかったりする理由は、50バイトで割り当てたつもりで他のリソースからfunc()にアクセスするコードを書いたことが原因でしょうか。

[yumetodo]

>あとおっしゃることを言い換えると、mallocで構造体のメモリサイズを確保する際は、ポインタ型にキャストすべきではないということですね。

そんな話はしていないと思う

[cateye]

>構造体とポインタの名前は分けるべき
は、可読性の問題なのでそうしなくてはいけないわけではありません。
>mallocで構造体のメモリサイズを確保する際は、ポインタ型にキャストすべきではない
違います！　malloc()が返すのはvoid*ですから構造体へのポインタへのキャストは必要です（aaa = (struct aaa*)malloc(sizeof(struct aaa)); ）。が、確保するメモリは構造体のサイズsizeof(struct aaa)です。
>動いたり動かなかったりする理由
malloc()が確保するのは一定量（４KBとか環境次第。ここから先はユーザか確保したサイズと言う意味ではなく実際にコンパイラがOSから取ってきて、割り当てた領域サイズの事です）です。それをmalloc()が呼ばれる度に切り崩して使います。（足りなくなれば、また確保する）なので、動く場合は確保した領域内にアクセスした場合で、動かないのは確保した領域をはみ出した場合です。こればかりはコンパイラの作りとOSで確保できるメモリサイズによります。

[imamoto_browser]

読み違えてました。malloc()の返り値はvoid*なので構造体へのポインタへのキャストが必要で、aaa = (struct aaa *) malloc(sizeof(struct aaa *));だとsizeof()の引数が構造体へのポインタのサイズで場合によってははみ出してしまうよってことでまずいってことですね。

[cateye]

yumetodoさんの回答にもありますが、ポインタ領域を確保してはダメと言うわけではありません。要は、何の領域を確保するのか？（構造体なのか構造体へのポインタなのか）・・・と言うことです。あと、C99以降は(struct aaa *)のポインタのキャストは不要ですね。（C++の影響でしょうか・・・反省）

[yumetodo]

>C++の影響でしょうか

むしろC++はその暗黙変換がないのでCより厳格かと