東京Ruby会議における
なるせさん の
A REINTRODUCTION TO RUBY M17Nで
我らCitrus Projectが「しっとりうす」に改名したことが発表されました(00:48:50〜)。
最近かぼすとかライムとか柑橘類の名を課したソフトを悪用して
タイーホされた事件との関連性を疑われることが多いため、今回の改名に至りました。
しっとり、うす。オカモト製ですねわかります。
ってまだ4/1には早いか。
つか、なるせさんの発表はたいへん素晴らしいので観てください。
というか今俺を悩ませてる某社製ライブラリの担当者、今直ぐ観て自分の誤りに気づいて直しやがれｗ
それと
噂の本もかなりの力作なのでこんなチラシの裏をsite:指定してググってる暇あるなら
是非買いましょう、この落書きにあるのは脱線だけですよー。
相変わらず予定は未定ちうことで、 前回の予告はまたまた後回しの手抜きバージョン。
前回はmbtowc(3)の使い方のまずさを指摘しましたが、今回はmbstowcs(3)です、
vi.cのvi_histedit()などで使われていますが、これもいまいちなコードですなぁ。
ざーっと何をやってるか説明しまひょ、el_line に含まれる一行分の
データ(wchar_t 形式で保持)を一時ファイルに書き出し、vi(1) を起動し編集が終わるまで待ちます。
編集が終わった後この一時ファイルを読み込んで、el_line のデータを更新するという簡単なお仕事。
ではコードを抜粋して処理を追ってみませう。
1020 len = (size_t)(el->el_line.lastchar - el->el_line.buffer);
1021 #define TMP_BUFSIZ (EL_BUFSIZ * MB_LEN_MAX)
1022 cp = el_malloc(TMP_BUFSIZ);
1023 if (cp == NULL)
1024 return CC_ERROR;
1025 line = el_malloc(len * sizeof(*line));
1026 if (line == NULL) {
1027 el_free((ptr_t)cp);
1028 return CC_ERROR;
1029 }
1030 Strncpy(line, el->el_line.buffer, len);
1031 line[len] = '\0';
1032 ct_wcstombs(cp, line, TMP_BUFSIZ - 1);
1033 cp[TMP_BUFSIZ - 1] = '\0';
1034 len = strlen(cp);
1035 write(fd, cp, len);
1036 write(fd, "\n", 1);
という処理です。
まぁ動けばおｋという悲観的なレビューならばこれでOK、しかしおいらが書くコードであれば
そもそもここでは wcstombs(3) は使わない使うべきではないという判断を下すでしょう。
というのも wcstombs(3) を使いたいあまりにとても無駄な富豪的コーディングになっているからです。
そもそもこの関数は NUL terminate されていることが前提の関数であるのに el_line.buffer から始まる
文字列の最後は改行コードですので、わざわざコピーせにゃ。わざわざ heap を？(なっち風)
*1。
またコピーに安全とはいえない関数 Strncpy() すなわち wcsncpy(3) を使っています。
1031行目で NUL terminate を明示的に付加してるので overrunすることはありませんが
なるべくならwcslcpy(3)のような安全な関数を使う癖をつけた方がいいでしょう。
多分 OpenBSD 方面だとぬっ頃されるコードその1。
このくそややこしいコードは簡潔に以下のように書き直せます
len = (size_t)(el->el_line.lastchar - el->el_line.buffer);
#ifdef WIDECHAR
int i;
mbstate_t st;
size_t len;
char buf[MB_LEN_MAX];
mbrtowc(NULL, NULL, 0, &st);
for (i = 0; i < len; ++i) {
len = wcrtomb(&buf[0], el->el_line.buffer[i], &st);
if (len == (size_t)-1)
return CC_ERROR;
write(fd, &buf[0], len);
}
#else
write(fd, el->el_line.buffer, len);
#endif
要するに malloc(3) イラネってことです、ただし安直にこのまま write(2) を繰り返し呼ぶと
system call の呼出回数が増えて性能が劣化しますので、バッファリングのコードを追加する必要がありますが
これもchar buf[4096] くらいしにしてループを一段足せばいいんじゃねぇのと。
そもそも本来ならこんなん自前でやらずにfwrite(3)を使えば勝手にバッファリングして
適度にwrite(2)を発行してくれるんですが…
お次は編集の終わった一時ファイルから読み込んで
1054 st = read(fd, cp, TMP_BUFSIZ);
1055 if (st > 0) {
1056 len = (size_t)(el->el_line.lastchar -
1057 el->el_line.buffer);
1058 len = ct_mbstowcs(el->el_line.buffer, cp, len);
1059 if (len > 0 && el->el_line.buffer[len -1] == '\n')
1060 --len;
1061 }
おーいおい、今度は read(2) した cp を NUL terminate してないので overrun するぜ(怒
mbstowcs(3)は第2引数の cp が'\0'に達するか、第3引数の len として渡したワイド文字列バッファ
(変換結果を保存、これは第1引数である el->el_line.buffer)の長さまで書き込まれたら終了です。
read(2) で上書きされなかった cp の中に残ったゴミ情報をワイド文字に変換しようとしてしまいますな。
まぁ len まで変換されれば止まるし、そもそも普通ゴミを変換すれば不正なマルチバイトと判定して
mbstowcs(3) は errno に EILSEQ をセットして止まってくれるので len まで進むことすらないので
overrun しても core 吐いて落ちるような事態にはなってないので発覚してないんでげそ(涙
こちらも前段のコードでmalloc(3)に依存しないコードに変更したので
#ifdef WIDECHAR
mbrtowc(NULL, NULL, 0, &ps);
for (i = 0; i < len; ++i) {
while ((nread = read(fd, buf, 1)) > 0) {
n = mbrtowc(&wc, &buf[0], 1, &ps);
if (n == (size_t)-1)
return CC_ERROR;
if (n != (size_t)-2)
break;
}
if (nread < 1 || wc == L'\n')
break;
el->el_line.buffer[i] = wc;
}
#else
read(fd, el->el_line.buffer, len);
#endif
というように書き直せます、まぁこっちも read(2) を1byteずつとゆーのも非効率ですので
バッファリング必要ですが。
そもそもそもそもですね、この部分って前回のコードとは異なり read/write 対象のファイルディスクリプタが
デバイスじゃなしに mkstemp(3) で作成した、ただのファイルであることが確定してるので
stdio.h の FILE を使用した高水準APIに書き直してもあんまり問題がないので心揺れるちゃうですよ。
動いてるものは触るな(なぜならそれは奇跡ry)の精神もだいぶ尽きてきたのと、
動いてないなら触れという話もあるので(ぉ
ただここは fork(2) も絡んでることだし fflush(3) 忘れなんかでバグを作り込みかねないので
とりあえずこのまま低水準APIを使ってバッファリングは自前で書くちう方針なのですが、あーどうしよう。
ま、実際に書き直したコードはまた次回ということで…