拾い物の関数を編集しているのですが、バグってしまいます。

[ロイヤー]

[拾い物の関数]

コード:

void makeNames(
    int makeNum, /* 作成数 */
    int wordNum, /* 単語数 */
    char * const words /* 単語列の領域 */
) {
    int wordPerName; /* １名前あたりの単語数 */
    char marks[] = ".-_"; /* 記号 */
    char numStr[ FIG_PER_NUM_MAX + 1 ]; /* 数字列の領域 */
    int i, j; /* 汎用カウンタ */

    /* 乱数を初期化する */
    srand( ( unsigned )time( NULL ) );
    for ( i = 0; i < 10; i ++ ) {
        rand();
    }

    /* 指定数だけ名前作成を繰り返す */
    for ( i = 0; i < makeNum; i ++ ) {
        printf( "%s", words + ( rand() % wordNum ) * WORD_LEN_MAX ); /* 最初の１語をランダムに決定・表示 */
        wordPerName = rand() % WORD_PER_NAME_MAX + 1; /* １名前あたりの単語数をランダムに決定 */
        if ( wordPerName <= 1 ) { /* 単語数が１（以下）なら */
            printf( "%s", makeRandNumStr( numStr, sizeof( numStr ) ) ); /* 数字列を後置する */
        }
        else {
            /* 単語数が１より大きければ、さらに単語を後置していく */
            for ( j = 1; j < wordPerName; j ++ ) {
                if ( rand() % 2 ) { /* 挟むのは数字列か記号かをランダムに決定 */
                    printf( "%s", makeRandNumStr( numStr, sizeof( numStr ) ) ); /* 数字列を挟む */
                }
                else {
                    printf( "%c", marks[ rand() % ( sizeof( marks ) - 1 ) ] ); /* 記号を挟む（-1 は終端文字を除外するため） */
                }
                printf( "%s", words + ( rand() % wordNum ) * WORD_LEN_MAX ); /* 後置する１語をランダムに決定・表示 */
            }
        }
        printf( "\n" ); /* １名前ごとに改行する */
    }
}

[編集後の関数]

コード:

void makeNames(char *const address,int wordNum,char *const words)
{
	char marks[] = "._-";
	char numStr[FIG_PER_NUM_MAX + 1];
	char *return_address;
	char str[100];

	int i,j;
	int wordPerName;

	return_address = address;

	for(i=0; i<10; i++)
	{
		rand();
	}

	//printf("%s",words + (rand() % wordNum) * WORD_LEN_MAX);
	strcat(return_address,words + (rand() % wordNum) * WORD_LEN_MAX);

	wordPerName = rand() % WORD_PER_NAME_MAX + 1;

	if(wordPerName <= 1)
	{
		strcat(return_address,makeRandNumStr(numStr,sizeof(numStr)));
		//printf("%s",makeRandNumStr(numStr,sizeof(numStr)));
	}
	else
	{
		for(j=1; j<wordPerName; j++)
		{
			if(rand() % 2)
			{
				strcat(return_address,makeRandNumStr(numStr,sizeof(numStr)));
				//printf("%s",makeRandNumStr(numStr,sizeof(numStr)));
			}
			else
			{
				strcat(return_address,marks[rand() % (sizeof(marks) -1)]);
				//printf("%c",marks[rand() % (sizeof(marks) -1)]);
			}

			strcat(return_address,words + (rand() % wordNum) * WORD_LEN_MAX);
			//printf("%s",words + (rand() % wordNum) * WORD_LEN_MAX);
		}
	}

	strcat(return_address,"\0");
	//printf("\n");
}

[拾い物の呼び出し]
makeNames( makeNum, wordNum, words );

[編集後の呼び出し]

コード:

	srand((unsigned)time(NULL));

	while(1)
	{
		address = NULL;

		makeNames(address,wordNum,words);

		printf("%s\n",address);

		getch();
	}

やりたいことは、拾い物の関数だと関数内でprintfを使用し表示していますが、
それを呼び出し側(main関数)で表示(引数で文字列をやりとり)させたいです。
コンパイルはエラーなく通るのですが、実行すると
http://gyazo.com/0f47e9602eaf17f62a9948a749c5a91e
このようなエラーウィンドウが表示されます。

なにかおかしいでしょうか？

[みけCAT]

なにかおかしいでしょうか？

はい。
サポートが終了したWindows XPを使っていると推測される
[編集後の関数]の19行目などでヌルポインタに対しstrcatで書き込もうとしているのがおかしいです。
[編集後の関数]のaddressにはあらかじめ十分な要素数を確保し、先頭にNUL文字を格納した領域へのポインタを渡すべきです。
また、[編集後の関数]の48行目のstrcatで0文字の文字列を連結しようとしているのも、ほとんど意味がなさそうであり、不自然です。

[box]

本題とは無関係だと思いますが…。

rand()の実行を10回ループしているのは何だか意味がなさそうに思います。
何のためのループでしょうか?

[ロイヤー]

[編集後の関数]の19行目などでヌルポインタに対しstrcatで書き込もうとしているのがおかしいです。
[編集後の関数]のaddressにはあらかじめ十分な要素数を確保し、先頭にNUL文字を格納した領域へのポインタを渡すべきです。
また、[編集後の関数]の48行目のstrcatで0文字の文字列を連結しようとしているのも、ほとんど意味がなさそうであり、不自然です。

呼び出し側関数(main)で最初に
char *address = NULL;
if(NULL == (address = malloc(300)))
{
printf("ERROR!!\n必要なメモリを確保できませんでした。\n");
return 0;
}
としてから
while(1)
{
address = NULL;

makeNames(address,wordNum,words);

printf("%s\n",address);

getch();
}
このように関数呼び出しなのですが、これではダメでしょうか？

[みけCAT]

呼び出し側関数(main)で最初に
char *address = NULL;
if(NULL == (address = malloc(300)))
{
printf("ERROR!!\n必要なメモリを確保できませんでした。\n");
return 0;
}
としてから
while(1)
{
address = NULL;

makeNames(address,wordNum,words);

printf("%s\n",address);

getch();
}
このように関数呼び出しなのですが、これではダメでしょうか？

ダメです。下のwhile文に入る前に確保した領域を開放していないのであれば、メモリリークが発生する可能性が非常に高いです。
(mallocが失敗すればメモリリークは起こらないでしょう)

[ロイヤー]

ダメです。下のwhile文に入る前に確保した領域を開放していないのであれば、メモリリークが発生する可能性が非常に高いです。
(mallocが失敗すればメモリリークは起こらないでしょう)

while文に入る前にadrressを解放(freeを使う)するってことですか？

今回のバグはメモリリークが原因なのでしょうか？

[みけCAT]

while文に入る前にadrressを解放(freeを使う)するってことですか？

いいえ。address(が指す領域)を(まだ使うのに)開放してはいけません。
makeNamesの呼び出しの直前にaddressにNULLを代入し、それをmakeNanesの第一引数に渡すのもやってはいけません。

今回のバグはメモリリークが原因なのでしょうか？

いいえ、書き込めない場所に書き込もうとしているのが原因です。

[ロイヤー]

いいえ。address(が指す領域)を(まだ使うのに)開放してはいけません。
makeNamesの呼び出しの直前にaddressにNULLを代入し、それをmakeNanesの第一引数に渡すのもやってはいけません。

addressにNULLを代入することで初期化(リセット)できると思っていたのですが、それが間違いってことですか？
どのように初期化すればよろしいですか？

いいえ、書き込めない場所に書き込もうとしているのが原因です。

①main関数内のmallocでメモリを確保
②そのアドレスをmakeNames関数に渡す
③return_address = address;でアドレスをコピー
④return_addressに文字列を代入
この使い方が間違っていますか？

[みけCAT]

addressにNULLを代入することで初期化(リセット)できると思っていたのですが、それが間違いってことですか？

間違っているかは何を初期化したいかによります。
ただし、メモリリークを起こす実装は間違いだと思います。

どのように初期化すればよろしいですか？

今回の場合、addressが指す領域にmemsetで0を書き込めばいいと思います。

①main関数内のmallocでメモリを確保
②そのアドレスをmakeNames関数に渡す
③return_address = address;でアドレスをコピー
④return_addressに文字列を代入
この使い方が間違っていますか？

間違ってはいないと思います。

[ロイヤー]

addressにNULLを代入することで初期化(リセット)できると思っていたのですが、それが間違いってことですか？

間違っているかは何を初期化したいかによります。
ただし、メモリリークを起こす実装は間違いだと思います。

どのように初期化すればよろしいですか？

今回の場合、addressが指す領域にmemsetで0を書き込めばいいと思います。

①main関数内のmallocでメモリを確保
②そのアドレスをmakeNames関数に渡す
③return_address = address;でアドレスをコピー
④return_addressに文字列を代入
この使い方が間違っていますか？

間違ってはいないと思います。

とりあえず

コード:

	while(1)
	{
		memset(address,0,sizeof(address));

		makeNames(address,wordNum,words);

		printf("%s\n",address);

		getch();
	}

このようにしました。

メモリリークの部分がいまいちわからないのですが

ダメです。下のwhile文に入る前に確保した領域を開放していないのであれば、メモリリークが発生する可能性が非常に高いです。
(mallocが失敗すればメモリリークは起こらないでしょう)

、
確保した領域とは

コード:

	if(NULL == (address = malloc(WORD_LEN_MAX * WORD_NUM_MAX)))
	{
		printf("ERROR!!\n必要なメモリを確保できませんでした。\n");
		fclose(inFile);
		return 0;
	}

この部分で確保した領域のことですか？
while文に入る前に領域解放とはどういうことでしょうか？

[みけCAT]

とりあえず

コード:

	while(1)
	{
		memset(address,0,sizeof(address));

		makeNames(address,wordNum,words);

		printf("%s\n",address);

		getch();
	}

このようにしました。

直接関係ないサイズを渡していて、たまたま動くとは思いますが怪しいですね。
無理に全体を初期化しようとせず、memsetの部分を

コード:

*address='\0';

とすると良いでしょう。

確保した領域とは

コード:

	if(NULL == (address = malloc(WORD_LEN_MAX * WORD_NUM_MAX)))
	{
		printf("ERROR!!\n必要なメモリを確保できませんでした。\n");
		fclose(inFile);
		return 0;
	}

この部分で確保した領域のことですか？

はい。

while文に入る前に領域解放とはどういうことでしょうか？

そのまんま、中でaddressが指す領域を利用するwhile文に入る前に、addressが指す領域を開放してしまう、ということです。

[ロイヤー]

直接関係ないサイズを渡していて、たまたま動くとは思いますが怪しいですね。
無理に全体を初期化しようとせず、memsetの部分を

コード:

*address='\0';

とすると良いでしょう。

コード:

	while(1)
	{
		*address = '\0';

		makeNames(address,wordNum,words);

		printf("%s\n",address);

		getch();
	}

こういうことですか？

そのまんま、中でaddressが指す領域を利用するwhile文に入る前に、addressが指す領域を開放してしまう、ということです。

addressが指す領域を解放するとは

コード:

free(address)

こういうことではないのですか？

[みけCAT]

コード:

	while(1)
	{
		*address = '\0';

		makeNames(address,wordNum,words);

		printf("%s\n",address);

		getch();
	}

こういうことですか？

はい。
実際にコンパイル・実行して、結果を報告していただけますか？

addressが指す領域を解放するとは

コード:

free(address)

こういうことではないのですか？

いいえ、そういうことです。

[ロイヤー]

はい。
実際にコンパイル・実行して、結果を報告していただけますか？

同じバグがでました。
http://gyazo.com/0f47e9602eaf17f62a9948a749c5a91e

いいえ、そういうことです。

while文の前にfreeしちゃっていいんですか！？

[みけCAT]

はい。
実際にコンパイル・実行して、結果を報告していただけますか？

同じバグがでました。
http://gyazo.com/0f47e9602eaf17f62a9948a749c5a91e

No: 1の[編集後の関数]の39行目で、strcatの第二引数にポインタとして無効な値を渡しているのが原因だと思います。
適当に補ってコンパイル可能にしたコードを貼っておきます。

► スポイラーを表示

コード:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>

#define FIG_PER_NUM_MAX 1024
#define WORD_LEN_MAX 10
#define WORD_PER_NAME_MAX 5
#define WORD_NUM_MAX 1024

char *makeRandNumStr(char *out, size_t bufferSize) {
	size_t i;
	if (out == NULL) return NULL;
	for (i = 0; i + 1 < bufferSize; i++) {
		out[i] = rand() % 10 + '0';
	}
	out[bufferSize - 1] = '\0';
	return out;
}

void getch(void) {
}

void makeNames(char *const address,int wordNum,char *const words)
{
	char marks[] = "._-";
	char numStr[FIG_PER_NUM_MAX + 1];
	char *return_address;
	char str[100];

	int i,j;
	int wordPerName;

	return_address = address;

	for(i=0; i<10; i++)
	{
		rand();
	}

	//printf("%s",words + (rand() % wordNum) * WORD_LEN_MAX);
	strcat(return_address,words + (rand() % wordNum) * WORD_LEN_MAX);

	wordPerName = rand() % WORD_PER_NAME_MAX + 1;

	if(wordPerName <= 1)
	{
		strcat(return_address,makeRandNumStr(numStr,sizeof(numStr)));
		//printf("%s",makeRandNumStr(numStr,sizeof(numStr)));
	}
	else
	{
		for(j=1; j<wordPerName; j++)
		{
			if(rand() % 2)
			{
				strcat(return_address,makeRandNumStr(numStr,sizeof(numStr)));
				//printf("%s",makeRandNumStr(numStr,sizeof(numStr)));
			}
			else
			{
				strcat(return_address,marks[rand() % (sizeof(marks) -1)]);
				//printf("%c",marks[rand() % (sizeof(marks) -1)]);
			}

			strcat(return_address,words + (rand() % wordNum) * WORD_LEN_MAX);
			//printf("%s",words + (rand() % wordNum) * WORD_LEN_MAX);
		}
	}

	strcat(return_address,"\0");
	//printf("\n");
}

int main(void) {
	char *address;
	FILE* inFile = stdin;
	int wordNum = 5;
	char * const words = (char * const)"asumi\0\0\0\0\0mizuhasi\0\0kitamura\0\0iguti\0\0\0\0\0horie\0\0\0\0\0";

	if(NULL == (address = malloc(WORD_LEN_MAX * WORD_NUM_MAX)))
	{
		printf("ERROR!!\n必要なメモリを確保できませんでした。\n");
		fclose(inFile);
		return 0;
	}

	srand((unsigned)time(NULL));

	while(1)
	{
		*address = '\0';

		makeNames(address,wordNum,words);

		printf("%s\n",address);

		getch();
	}

	return 0;
}

このコードをWandboxでコンパイルすると、

コード:

prog.c: In function 'makeNames':
prog.c:62:5: warning: passing argument 2 of 'strcat' makes pointer from integer without a cast [enabled by default]
     strcat(return_address,marks[rand() % (sizeof(marks) -1)]);
     ^
In file included from prog.c:3:0:
/usr/include/string.h:136:14: note: expected 'const char * restrict' but argument is of type 'char'
 extern char *strcat (char *__restrict __dest, __const char *__restrict __src)
              ^
prog.c:29:7: warning: unused variable 'str' [-Wunused-variable]
  char str[100];
       ^

という警告が出ました。

while文の前にfreeしちゃっていいんですか！？

ちゃんとダメって書きました。

while文に入る前にadrressを解放(freeを使う)するってことですか？

いいえ。address(が指す領域)を(まだ使うのに)開放してはいけません。
makeNamesの呼び出しの直前にaddressにNULLを代入し、それをmakeNanesの第一引数に渡すのもやってはいけません。

[ロイヤー]

このコードをWandboxでコンパイルすると、

コード:

prog.c: In function 'makeNames':
prog.c:62:5: warning: passing argument 2 of 'strcat' makes pointer from integer without a cast [enabled by default]
     strcat(return_address,marks[rand() % (sizeof(marks) -1)]);
     ^
In file included from prog.c:3:0:
/usr/include/string.h:136:14: note: expected 'const char * restrict' but argument is of type 'char'
 extern char *strcat (char *__restrict __dest, __const char *__restrict __src)
              ^
prog.c:29:7: warning: unused variable 'str' [-Wunused-variable]
  char str[100];
       ^

という警告が出ました。

VC++2010を使っているのですが、
>test.c(125) : warning C4047: '関数' : 間接参照のレベルが 'const char *' と 'char
' で異なっています。
>test.c(125) : warning C4024: 'strcat' : の型が 2 の仮引数および実引数と異なりま
す。
と、同じような警告が出ていました。
普通に実行ファイルが作られたので、そのまま実行しようとしていましたが、警告は守らないといけないですね。
これはどのように対処すればいいのですか？
第二引数をキャストすることで警告は一応消えますが、それは間違いですよね？

ちゃんとダメって書きました。

ですよね。
私もそれがダメなのは理解しているのですが、

中でaddressが指す領域を利用するwhile文に入る前に、addressが指す領域を開放してしまう、ということです。

このwhile文に入る前に領域を解放というのがまだよくわからないのですが。。。

[みけCAT]

VC++2010を使っているのですが、
>test.c(125) : warning C4047: '関数' : 間接参照のレベルが 'const char *' と 'char
' で異なっています。
>test.c(125) : warning C4024: 'strcat' : の型が 2 の仮引数および実引数と異なりま
す。
と、同じような警告が出ていました。
普通に実行ファイルが作られたので、そのまま実行しようとしていましたが、警告は守らないといけないですね。
これはどのように対処すればいいのですか？

例えば、このstrcatchr関数をmakeNames関数の前に追加し、そこのstrcatのかわりにstrcatchrを使えばいいです。

コード:

char* strcatchr(char *str, char chrToAdd) {
	size_t pos;
	if (str == NULL) return NULL;
	pos = strlen(str);
	str[pos] = chrToAdd;
	str[pos + 1] = '\0';
	return str;
}

第二引数をキャストすることで警告は一応消えますが、それは間違いですよね？

はい。

ちゃんとダメって書きました。

ですよね。
私もそれがダメなのは理解しているのですが、

中でaddressが指す領域を利用するwhile文に入る前に、addressが指す領域を開放してしまう、ということです。

このwhile文に入る前に領域を解放というのがまだよくわからないのですが。。。

while文に入る前に領域を解放というのは、while文に入る前に領域を解放という意味しか無いのですが…
申し訳ありませんが、本題にあまり関係ないので忘れていただけますか？

[ロイヤー]

例えば、このstrcatchr関数をmakeNames関数の前に追加し、そこのstrcatのかわりにstrcatchrを使えばいいです。

コード:

char* strcatchr(char *str, char chrToAdd) {
	size_t pos;
	if (str == NULL) return NULL;
	pos = strlen(str);
	str[pos] = chrToAdd;
	str[pos + 1] = '\0';
	return str;
}

あー、なるほど。
そういうことですか。

while文に入る前に領域を解放というのは、while文に入る前に領域を解放という意味しか無いのですが…
申し訳ありませんが、本題にあまり関係ないので忘れていただけますか？

理解力がなくてすみません。。。
もっと勉強します。


ありがとうございました。
もっともっと勉強して人が作ったコードも自分で編集できるように頑張ります。