プログラマ専用SNS ミクプラ

ロイヤー さんが書きました:なにかおかしいでしょうか？

みけCAT さんが書きました: [編集後の関数]の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 さんが書きました: ダメです。下のwhile文に入る前に確保した領域を開放していないのであれば、メモリリークが発生する可能性が非常に高いです。
(mallocが失敗すればメモリリークは起こらないでしょう)

ロイヤー さんが書きました:while文に入る前にadrressを解放(freeを使う)するってことですか？

ロイヤー さんが書きました:今回のバグはメモリリークが原因なのでしょうか？

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

みけCAT さんが書きました: いいえ、書き込めない場所に書き込もうとしているのが原因です。

ロイヤー さんが書きました:addressにNULLを代入することで初期化(リセット)できると思っていたのですが、それが間違いってことですか？

ロイヤー さんが書きました:どのように初期化すればよろしいですか？

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

みけCAT さんが書きました:

ロイヤー さんが書きました: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();
	}

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

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

ロイヤー さんが書きました:とりあえず

コード:

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

		makeNames(address,wordNum,words);

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

		getch();
	}

このようにしました。

*address='\0';

ロイヤー さんが書きました:確保した領域とは

コード:

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

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

ロイヤー さんが書きました:while文に入る前に領域解放とはどういうことでしょうか？

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

コード:

*address='\0';

とすると良いでしょう。

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

		makeNames(address,wordNum,words);

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

		getch();
	}

みけCAT さんが書きました: そのまんま、中でaddressが指す領域を利用するwhile文に入る前に、addressが指す領域を開放してしまう、ということです。

free(address)

ロイヤー さんが書きました:

コード:

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

		makeNames(address,wordNum,words);

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

		getch();
	}

こういうことですか？

ロイヤー さんが書きました:addressが指す領域を解放するとは

コード:

free(address)

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

みけCAT さんが書きました: はい。
実際にコンパイル・実行して、結果を報告していただけますか？

みけCAT さんが書きました: いいえ、そういうことです。

ロイヤー さんが書きました:

みけCAT さんが書きました: はい。
実際にコンパイル・実行して、結果を報告していただけますか？

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

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

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しちゃっていいんですか！？

みけCAT さんが書きました:

ロイヤー さんが書きました:while文に入る前にadrressを解放(freeを使う)するってことですか？

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

みけCAT さんが書きました: このコードを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];
       ^

という警告が出ました。

みけCAT さんが書きました: ちゃんとダメって書きました。

みけCAT さんが書きました: 中でaddressが指す領域を利用するwhile文に入る前に、addressが指す領域を開放してしまう、ということです。

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

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

ロイヤー さんが書きました:第二引数をキャストすることで警告は一応消えますが、それは間違いですよね？

ロイヤー さんが書きました:

みけCAT さんが書きました: ちゃんとダメって書きました。

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

みけCAT さんが書きました: 中でaddressが指す領域を利用するwhile文に入る前に、addressが指す領域を開放してしまう、ということです。

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

みけCAT さんが書きました: 例えば、この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;
}

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