Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

変換先バッファ確保の new で例外処理を行わないように std::nothrow 指定追加 #287

Merged
merged 1 commit into from
Aug 14, 2018

Conversation

beru
Copy link
Contributor

@beru beru commented Jul 20, 2018

new で例外が起きたら catch で捕まえてポインタに NULL を設定する記述を std::nothrow 指定するように置き換えました。

new している箇所は他にもいっぱいありますが、それらについてどうしてるのかは良く分かっていません。

@berryzplus
Copy link
Contributor

コードを触るより先に、どうあるべきかを話たほうが良さそうな話題です。
すでに issue #53 が立ってますし。

ぼくの認識

  • 普通のnew 失敗したら std::bad_alloc例外。std::unexpectedが呼ばれる?
  • nothrow new 失敗したらabort。std::terminateが呼ばれる

このへん詳しくはないので、用語は間違ってるかも知れません。

@beru
Copy link
Contributor Author

beru commented Jul 21, 2018

そういえば #53 がありましたね。そこでちゃんと議論した方が良いんでしょうか?

下記のプログラムを書いて調べてみました。

#include <stdio.h>
#include <new>
#include <iostream>

void test_new(size_t sz)
{
	std::cout << "sz : " << sz/1024/1024 << "MiB" << std::endl;
	// try catch 使用
	{
		char* p1 = nullptr;
		char* p2 = nullptr;
		try {
			p1 = new char[sz];
		} catch (std::bad_alloc& e) {
			std::cout << "p1: " << e.what() << std::endl;
		}
		try {
			p2 = new char[sz];
		} catch (std::bad_alloc& e) {
			std::cout << "p2: " << e.what() << std::endl;
		}
		delete[] p1;
		delete[] p2;
	}
	// nothrow 使用
	{
		char* p1 = nullptr;
		char* p2 = nullptr;
		p1 = new (std::nothrow) char[sz];
		if (!p1) {
			std::cout << "p1 new failed." << std::endl;
		}
		p2 = new (std::nothrow) char[sz];
		if (!p2) {
			std::cout << "p2 new failed." << std::endl;
		}
		delete[] p1;
		delete[] p2;
	}
	std::cout << std::endl;
}

int main(int argc, char* argv[])
{
	test_new(1U << 29);
	test_new(1U << 30);
	test_new(1U << 31);
	if (sizeof(size_t) == 8) {
		test_new((size_t)(1ULL << 32));
		test_new((size_t)(1ULL << 33));
		test_new((size_t)(1ULL << 34));
		test_new((size_t)(1ULL << 35));
	}
	
	return 0;
}

x86 Release での実行結果

sz : 512MiB

sz : 1024MiB
p2: bad allocation
p2 new failed.

sz : 2048MiB
p1: bad allocation
p2: bad allocation
p1 new failed.
p2 new failed.

続行するには何かキーを押してください . . .

x64 Release での実行結果

sz : 512MiB

sz : 1024MiB

sz : 2048MiB

sz : 4096MiB

sz : 8192MiB

sz : 16384MiB
p2: bad allocation
p2 new failed.

sz : 32768MiB
p1: bad allocation
p2: bad allocation
p1 new failed.
p2 new failed.

続行するには何かキーを押してください . . .

@beru
Copy link
Contributor Author

beru commented Jul 21, 2018

という事で new(std::nothrow) の場合にメモリ確保に失敗したら nullptr が返ってくるようです。
他の挙動が起きるのかどうかは分かっていません。

https://en.cppreference.com/w/cpp/memory/new/nothrow

あとこのPRの目的としては一部のコードの記述をシンプルにする事で、(メモリ確保失敗時の挙動を踏まえて)全般的に new でメモリ確保している箇所の記述を直すつもりはありません。それは大変そうなので…。

@uzanka
Copy link

uzanka commented Jul 21, 2018

delete[] p1;
delete[] p2;

@beru
Copy link
Contributor Author

beru commented Jul 21, 2018

delete[] p1;
delete[] p2;

ご指摘ありがとうございます。。
delete[] と書かないと(先頭の要素だけしか)デストラクタが呼ばれないというのがありますが、今回のケースではPOD型なので問題は起きないと思います。ただ記述として宜しくないですね。修正しておきます。

Copy link
Contributor

@berryzplus berryzplus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTMです。

変更前後で変数名、型、要求サイズに差異がないことを確認しました。

@berryzplus
Copy link
Contributor

チェック資材

diff.zip

@berryzplus berryzplus merged commit 682965a into sakura-editor:master Aug 14, 2018
@berryzplus
Copy link
Contributor

とくに文句も出てないのでマージしちゃいました。

@beru beru deleted the nothrow branch August 14, 2018 20:41
@m-tmatma m-tmatma added this to the next release milestone Aug 19, 2018
HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 11, 2019
変換先バッファ確保の new で例外処理を行わないように std::nothrow 指定追加
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants