ラムダ式の変数キャプチャでの注意点
今回はプログラミング中に原因が分かるまでかなり時間がかかったバグについて話そうと思います。
もちろん色々伏せていますが、まずはファイルを作成して、doSomethingWithFile1() と doSomethingWithFile2() にそのファイルを渡す以下のような関数を書いていました。
int createFileAndDoSomething() {
int ret = 0;
const std::string file_to_path("/path/to/file");
ret = create(file_to_path);
if (ret < 0) {
printf("createFile(%s) failed ret = %d\n", file_to_path.c_str(), ret);
return ret;
}
ret = doSomethingWithFile1(file_to_path);
if (ret < 0) {
printf("doSomethingWithFile1(%s) failed ret = %d\n", file_to_path.c_str(), ret);
return ret;
}
ret = doSomethingWithFile2(file_to_path);
if (ret < 0) {
printf("doSomethingWithFile2(%s) failed ret = %d\n", file_to_path.c_str(), ret);
return ret;
}
return 0;
}
でもdoSomethingWithFile1()とdoSomethingWithFile2()のどちらかが失敗したら、作成したファイルを削除しないといけないことに気づいて、コードの複製を避けるために、ファイルを削除するラムダ式関数を追加しました。
int createFileAndDoSomething() {
int ret = 0;
const std::string file_to_path("/path/to/file");
ret = create(file_to_path);
if (ret < 0) {
printf("createFile(%s) failed ret = %d\n", file_to_path.c_str(), ret);
return ret;
}
auto cleanUp
( [&] {
ret = removeFile(file_to_path);
if (ret < 0)
{
printf("removeFile(%s) failed ret = %d\n", file_to_path.c_str(), ret);
}
return 0;
});
ret = doSomethingWithFile1(file_to_path);
if (ret < 0) {
printf("doSomethingWithFile1(%s) failed ret = %d\n", file_to_path.c_str(), ret);
cleanUp();
return ret;
}
ret = doSomethingWithFile2(file_to_path);
if (ret < 0) {
printf("doSomethingWithFile2(%s) failed ret = %d\n", file_to_path.c_str(), ret);
cleanUp();
return ret;
}
return 0;
}
ファイルの削除が失敗したとしても、特にエラーにしない仕様になっています。
「これで完璧だ！」と思って、doSomethingWithFile1()が失敗する場合ファイルが削除されることを確認しようとしたら、失敗した時の以下のログは出ました。
doSomethingWithFile1(/path/to/file) failed ret = -1
でもなぜかcreateFileAndDoSomething()という関数自体が成功したように、プログラムが普通に続いていました。
「あれ？なんでだ？」と思って、動作確認を繰り返してみても結果が変わらないので、コードをじっくり見てみたら、cleanUp()の中でdoSomethingWithFile1()のエラーコードが入っているretをremoveFile()の戻り値で上書きしてしまっていました。
以下の方が正しいです。
auto cleanUp
( [&] {
const int ret2 = removeFile(file_to_path);
if (ret2 < 0)
{
printf("removeFile(%s) failed ret = %d\n", file_to_path.c_str(), ret2);
}
return 0;
});
ラムダ式関数の[&]ですべてのメンバー変数とローカルの変数に引数として定義せずにアクセスできるのは凄く便利なのですが、アクセスしない方がいいものにもアクセスできてしまうから、こういうミスが起こるかもしれないです。
そもそもこういう戻り値用の変数の使いまわす時は注意しないといけないですね。
後、cleanUp()を何回もコールするのを避けたい場合は、逆にdoSomethingWithFile1()とdoSomethingWithFile2()の方をラムダ式関数に移動させるという以下の手もあります。
int createFileAndDoSomething() {
int ret = 0;
const std::string file_to_path("/path/to/file");
ret = create(file_to_path);
if (ret < 0) {
printf("createFile(%s) failed ret = %d\n", file_to_path.c_str(), ret);
return ret;
}
auto doSomethingWithFile
( [&] {
ret = doSomethingWithFile1(file_to_path);
if (ret < 0) {
printf("doSomethingWithFile1(%s) failed ret = %d\n", file_to_path.c_str(), ret);
return ret;
}
ret = doSomethingWithFile2(file_to_path);
if (ret < 0) {
printf("doSomethingWithFile2(%s) failed ret = %d\n", file_to_path.c_str(), ret);
return ret;
}
return 0;
});
ret = doSomethingWithFile();
if (ret < 0) {
printf("doSomethingWithFile() failed ret = %d\n", ret);
const int ret2 = removeFile(file_to_path);
if (ret2 < 0) {
printf("removeFile(%s) failed ret2 = %d\n", file_to_path.c_str(), ret2);
}
}
return 0;
}
戻り値用の変数の使いまわしとラムダ式関数の中での変数の上書きに気をつけましょう、という話でした
Discussion