2009-07-15

イディオムはセオリー通りに書かないとだめ

気づいてしまえば非常につまらないミスなんですが。

void some_class::some_method(int parameter)
{
  int p;
  while ( p = m_pClass->some_enumerator(parameter) < 0 ) {
    do_some_work(p);
  }
}

うんうん、m_pClassの指しているオブジェクトがなにかコレクションを保持していて、some_enumerator()で条件を指定して何かを数え上げるのね。それでもう何もなくなれば負値を返すので、取得できたものについてdo_some_work()で処理をすると。...が、動かない。なにも起こらない。

よく見ればわかりますが、ここでやりたかったことでは、whileの条件式は

  while ((p = m_pClass->some_enumerator(parameter)) < 0) {

と代入演算子側をカッコでくくらないとだめです。一つ上の書き方では、'<'の優先順位の方が上なので、「数え上げるものがある間、pに'1'を代入し続ける」動作になります。

ここでのみそは、どちらの書き方でもループ回数は変わらないところでしょうか。今回、上で言うdo_some_work()が、たまたま'1'だけ渡しても一見何も起こらないだけなので、すぐには気付きませんでした。落ちたり無限ループにでもなれば、さすがにすぐおかしいと思うのでしょうが。

2008-10-11

チョコシナモンシフォンケーキ

甘味

池袋に来たので、例によって椿屋。
ケーキセットで上記のものを。

シナモンの香りがいいです。

2008-10-09

ぶどう狩りに行ってきました

食べ物

この間の日曜日、山梨にぶどう狩りに行ってきました。

甲府の「早川園」です。
今回学んだ事:

JR善光寺駅は、長野の善光寺と違う。
見延線に乗る時は、所定の位置に並んで、電車が止まったら開扉ボタンを押さないといけない。
見延線は、甲府方面が下り。

2008-03-23

ベースクラスの関数シグネチャを不用意に変えてはいけない

プログラミング C++

会社で実際にあったバグ。とりあえずこんなふうに継承関係のあるクラスがありました。

class CBaseClass {
public:
	CBaseClass() { }
	~CBaseClass() { }

	virtual void TestFunc() {
		std::cout << "CBaseClass::TestFunc() called.\n";
	}
};

class CDerivedClass : public CBaseClass {
public:
	CDerivedClass() { }
	~CDerivedClass() { }

	virtual void TestFunc() {
		std::cout << "CDerivedClass::TestFunc() called.\n";
	}
};

ほとんど同じなんだけど、最後のステップだけ違う処理をする2つのクラス。違う処理をするメソッドだけ、仮想関数にして差し替えることで、実装を共通化してありました。そして呼び出し側では、

...
	CBaseClass baseObj;
	CDerivedClass derivedObj;
	CBaseClass* pBase;
...
	if (some condition) {
		pBase = &baseObj;
	}
	else {
		pBase = &derivedObj;
	}
...
...
	pBase->TestFunc();

こんなふうに、最初の条件チェックでどちらかのクラスのオブジェクトへのポインタを作成しておいて、共通の処理を実行、最後に仮想関数呼び出しで適切な後処理をする、というコードです。

ところが、いつのバージョンからか、2つのうち最初のクラスの後処理だけ、さらに条件分岐する処理が追加されていました。

class CBaseClass {
public:
	CBaseClass() { }
	~CBaseClass() { }

	virtual void TestFunc(int param = 0) {  //引数'param'を追加（でも元と同じ引数なしでも使う）
		std::cout << "CBaseClass::TestFunc(" << param << ") called.\n";
	}
};

デフォルト引数を与えてあるので、コードのほかの部分には影響ないよねー、ということだったと思うのですが、どうにも変な動きをします。デバッガで追ってみると、「2つのクラスで違う処理をする」TestFunc()が、必ずCBaseClass::TestFunc()に分岐してしまっています。

	pBase->TestFunc(); //必ずCBaseClass::TestFunc()が呼ばれるよ！

結局、CBaseClass::TestFunc()に引数を加えたことで、TestFunc()のメソッドシグネチャが変わってしまい、CDerivedClass::TestFunc()がCBaseClass::TestFunc()をオーバーライドしなくなってしまっていたのでした。

void CBaseClass::TestFunc(int param = 0); //この2つは
void CDrivedClass::TestFunc();            //まったく別のメソッド

それで、冒頭のベースクラスの関数シグネチャを不用意に変えてはいけないになるのですが...
この「デフォルト引数を除くと同じ名前のメソッドが、継承関係のあるクラスでそれぞれvirtual宣言されている」という状況、C++の文法的には全く問題がないのですね。
VC++で警告レベルを/W4にしても、何の警告も出ません（ためしにgccで-Wallとしてもやってみましたが同じでした）。

まあ、単純ミスといえば単純ミスなんですが、C#のvirtual/overrideキーワードをそれぞれ指定する文法だとエラー検出できるよね、とか、そもそもCBaseClassとCDerivedClassから共通の抽象クラスを抽出する設計にすべきだったんじゃ、とかいろいろ考えてしまいました。

でも今回のケースでは、（このコードが実装しようとしていた）元の仕様が無駄に複雑すぎるのが一番の問題、というオチだったりして。

2008-02-17

一部の日本語フォルダ名でインデックスが作成されない件、とその回避パッチ

HyperEstraier オープンソースプログラミング

結論

Hyper Estraier バージョン1.4.13を日本語Windows環境で使用した場合：

estcmd gatherの検索対象に、0x5cで終わるフォルダ名があると、そのフォルダ以下はインデックスが作成されない。このため、検索漏れが生じる。
原因は、estcmdのディレクトリスキャンルーチンのコードと、MinGWのopendir()の実装がマルチバイト用の文字処理ルーチンを呼んでいないことの２つ。
estcmd.cに対するワークアラウンドパッチ

勤め先の部門サーバに、HyperEstraierによる全文検索システムを導入しました。すると、使った人から「入れたはずの価格表が検索にかからない」との指摘。ログを確認してみると、”xxx価格表”サブディレクトリ以下がごっそりスキャンすらされていません。なんでだーと思い、じっと眺めていると、...\xxx価格表\hoge.txt...
表という文字がなんかいかにも怪しそうです。

現象確認

試しに単純なディレクトリ構造を作り、スキャンだけやってみると：

C:\tmp>tree /F c:\tmp\testest
フォルダ パスの一覧
ボリューム シリアル番号は xxxx-xxxx です
C:\TMP\TESTEST
├─テスト
│      価格表.txt
│
└─テスト表
        その他価格表.txt

C:\tmp>estcmd scandir c:\tmp\testest
c:\tmp\testest
c:\tmp\testest\テスト
c:\tmp\testest\テスト\価格表.txt
c:\tmp\testest\テスト表
c:\tmp\testest\テスト表テスト表

やはり、”テスト表”サブディレクトリ以下は正しく読まれていません。おそらくディレクトリスキャンルーチンの文字列処理だけだろうとあたりをつけて、調べてみることにしました（先日の再ビルドする必要→d:id:aenomoto:20080210、はこのためでした）。

修正点１

まず、scandirサブコマンドを実行しているディレクトリスキャンルーチンに以下の箇所があります。

estcmd.c, procscandir():

  path = (len > 0 && line[len-1] == ESTPATHCHR) ? cbsprintf("%s%s", line, tmp) :
    cbsprintf("%s%c%s", line, ESTPATHCHR, tmp);

ディレクトリ名とファイル名を繋げて、パス名を組み立てているのですが、間にディレクトリ区切り文字（'\'）をはさむかどうかの判定に、ディレクトリ名文字列の最終バイトを直接'\'と比較しています。これだと、ディレクトリ名が日本語で最終文字の2バイト目が0x5cの場合と、本当に半角の'\'で終了している場合と、区別することができません。
（2バイト目が0x5cの文字で有名なやつの一つが’表’です。参考→http://katsura-kotonoha.sakura.ne.jp/prog/etc/tip00001.shtml）

以下のように書き換えます。

  unsigned char* _mbsrchr(const unsigned char* string, unsigned int c);

  if (len > 0 && (_mbsrchr(line, ESTPATHCHR) == line + len - 1)) {
    path = cbsprintf("%s%s", line, tmp);
  }
  else {
    path = cbsprintf("%s%c%s", line, ESTPATHCHR, tmp);
  }

ディレクトリ名の末尾がパス名区切り文字かは、strrchr()で最期に出現する'\'文字の位置を探索し、それが文字列の最終バイトかどうかで判断します。_mbsrchr()はVC++ランタイムの拡張関数で、strrchr()のマルチバイト版です。MinGWのバイナリは、最終的にはVC++ランタイムにリンクされるので、きちんとプロトタイプ宣言してあげればそのまま使えるはずです。

上記の処理はestcmd.c中に2ヶ所あります（scandir/gather）。それぞれを上記のコードに差し替えます。

修正点2

以上の修正で"テスト表"サブディレクトリ後の'\'が落ちる現象はなくなりますが、まだその下のファイルを正しく検出できません。

これは、MinGWのUNIX互換ディレクトリスキャンルーチンの実装に問題があるようです。opendir()/readdir()を呼び出すだけの単純なプログラムでも（こんなの→test-opendir.c）：

>test-opendir.exe C:\tmp\testest\テスト
d_name=>'.'
d_name=>'..'
d_name=>'価格表.txt'

>test-opendir.exe C:\tmp\testest\テスト表
d_name=>'テスト表'

となってしまいます。
MinGWでopendir()/readdir()を実装しているランタイムライブラリのソースを確認してみます。mingw-runtime-3.14-src.tar.gzの内容によれば、

mingwex/dirent.c, _topendir():
...
  /* Add on a slash if the path does not end with one. */
  if (nd->dd_name[0] != _T('\0')
      && _tcsrchr (nd->dd_name, _T('/')) != nd->dd_name
					    + _tcslen (nd->dd_name) - 1
      && _tcsrchr (nd->dd_name, _T('\\')) != nd->dd_name
      					     + _tcslen (nd->dd_name) - 1)
    {
      _tcscat (nd->dd_name, SLASH);
    }
...

となっていて、修正点１と同じ手法で引数文字列がパス名区切り文字で終わっているか判定し、必要なら付加しています（というか、このコードを見て修正点１のコードを書いたのですが:-)。
ここで_tcsrchr()はWindowsの汎用文字型TCHAR用のstrrchr()ですが、実際の置き換えは（当然）Microsoftのヘッダではなく、独自のtchar.hで行われています。

include/tchar.h:

#ifdef _UNICODE
#define	_tcsrchr	wcsrchr
#else
#define	_tcsrchr	strrchr
#endif

これ、確かにMicrosoftのコードと同じ挙動ですが、MicrosoftのTCHARルーチンは、さらに_MBCSマクロの影響も受けます。概念的には、

#ifdef _UNICODE
# define	_tcsrchr	wcsrchr
#else
# ifdef _MBCS
#  define	_tcsrchr	_mbsrchr
# else
#  define	_tcsrchr	strrchr
# endif
#endif

結局、MinGWランタイムのopendir()はパス名区切り文字の検索にマルチバイトには非対応のstrrchr()を使用し、修正1と同じ理由で"テスト表"フォルダのパス名区切り付加に失敗します。MinGWのopendir()は、この後さらに'*'を付加してAPIによるディレクトリ探索を実行しているので、正しいファイルリストは得られないことになります。

これの修正は、MinGWランタイムを日本語Windowsではマルチバイト版の文字列処理関数を呼ぶようにできればいいのでしょうが、それはちょっと面倒そうです。

上記の処理は、opendir()の引数として渡された文字列が'\'で終端されていない場合の処理です。ということは、opendir()呼び出し時に、引数の末尾に必ず'\'が付くようにしてあげればこの問題を回避できます。

HyperEstraier側で実際にopendir()を呼び出しているのは、本体ではなくQDBMです。QDBMにディレクトリリストを作成するサービスルーチンがあり、

cabin.c, cbdirlist():

CBLIST *cbdirlist(const char *name){
...
  assert(name);
  if(!(DD = opendir(name))) return NULL;
  CB_LISTOPEN(list);
...

となっています。引数のnameは直接opendir()に渡されているので、HyperEstraier本体のcbdirlist()呼び出し時に、ディレクトリ名末尾に'\'を付加するようにしてしまいましょう。

CBLIST *mbwa_cbdirlist(const char *name)
{
...
  //ディレクトリ名に細工する
...
  return cbdirlist(name);
}

...こんな感じに。estcmd.c内でcbdirlist()の呼び出しをこの関数に置き換えれば、ワークアラウンドパッチの対象を小さく、estcmd.cファイルひとつだけにできます。

というわけで、以上2つの修正を組み合わせたパッチ→hyperestraier-1.4.13-mbfs-workaround.patch

再テスト

C:\tmp>estcmd scandir c:\tmp\testest
c:\tmp\testest
c:\tmp\testest\テスト
c:\tmp\testest\テスト\価格表.txt
c:\tmp\testest\テスト表
c:\tmp\testest\テスト表\その他価格表.txt

これで、サーバにある、なんとか価格表だの、なんたら予定表だのといったフォルダも検索できるようになりました。

2008-02-10

Hyper EstraierのMinGW上での再ビルド

HyperEstraier オープンソースプログラミング

会社の部内サーバ検索用に、オープンソースの全文検索システム、HyperEstraierを導入してみました。簡単にセットアップでき、検索も高速でとてもいいですね！作者の方に感謝です。
バイナリパッケージも配布されていますが、今回、必要があってWindows用バイナリの再ビルドを行ったので、手順をメモしておきます。

以下の手順については、こちらのページが大変参考になりました：しののんだいあり〜[pc] HyperEstraierのビルド

使用したパッケージ

hyperestraier-1.4.13.tar.gz
qdbm-1.8.77.tar.gz
MinGW-5.1.3.exe
MSYS-1.0.10.exe
libiconv-1.11-1-bin.tar.bz2
libiconv-1.11-1-dll.tar.bz2
mingw-libgnurx-2.5.1-bin.tar.gz
mingw-libgnurx-2.5.1-dev.tar.gz
pthreads-w32-2-8-0-release.exe
zlib123-dll.zip
UnxUtils.zip
d2txt127.zip

配布元サイト

ツールと依存パッケージのインストール

(1)MinGWとMSYSのインストール。MinGWは、パッケージ選択でg++ compilerのみを選択する。
(2)MinGWの追加ライブラリをインストール。アーカイブファイルを展開して以下の各所にコピー。MSYSはインストール先のbin,doc,etcなどのあるディレクトリが、ルートと/usrの両方に見える。したがって、msys/1.0/local以下に各ファイルを配置することで/usr/local/*へインストールしたことになる。

アーカイブ	展開したファイル	コピー先
libiconv-1.11-1-bin.tar.bz2	usr/local/*	msys/1.0/local/*
libiconv-1.11-1-dll.tar.bz2	usr/local/bin/*	msys/1.0/local/bin/*
mingw-libgnurx-2.5.1-bin.tar.gz	bin/*	msys/1.0/local/bin/*
mingw-libgnurx-2.5.1-dev.tar.gz	mingw-libgnurx-2.5.1-dev/*	msys/1.0/local/*
pthreads-w32-2-8-0-release.exe	Pre-built.2/include/* Pre-built.2/lib/*		msys/1.0/local/include/* msys/1.0/local/lib/*
zlib123-dll.zip	zlib1.dll include/* lib/*	msys/1.0/local/bin/zlib1.dll msys/1.0/local/include/* msys/1.0/local/lib/*
UnxUtils.zip	usr/local/wbin/zip.exe	msys/1.0/local/bin/zip.exe
d2txt127.zip	xdoc2txt.*, zlib.dll	msys/1.0/local/xdoc2txt/*

zlib123-dllは、ファイルのコピー後、MSYSのコンソールを起動し、USAGE.txtの記述にしたがって、

cd /usr/local
dlltool.exe -D bin/zlib1.dll -d lib/zlib.def -l lib/libzdll.a

qdbmのビルド/インストール

(1)msys/1.0/home/<ユーザ名>/以下にqdbm-1.8.77.tar.gzを展開（MSYSのコンソールで、ホームディレクトリになる）。
(2)configureスクリプト、Makefile.inを若干編集する。

圧縮ライブラリのリンクオプションが-lzになっているので、-lzdllに書き換える（３ヶ所）。
MinGW用ターゲット('mingw:')のコマンドラインに、zlibとiconvの参照先ディレクトリを追記する。makeの引数で設定しているLIBLDFLAGS/LDFLAGSの２つのマクロの先頭に、'-L/usr/local/lib'を追加。

パッチファイルにまとめるとこんな感じ→QDBMをMinGWで再ビルドするときの変更点
(3)以下のコマンドでビルド&インストール。

cd qdbm-1.8.77
patch -p1 < ../-1.8.77-build-on-mingw.patch
./configure --enable-zlib --enable-iconv
make mingw
make install-win

これで/usr/local/以下にインストールされる。

'--enable-zlib'で圧縮を有効にしないと、公式配布のバージョンで作成したインデックスが使用できなくなってしまう。

HyperEstraierのビルド

(1)msys/1.0/home/<ユーザ名>/以下にhyperestraier-1.4.13.tar.gzを展開。
(2)configureスクリプトを若干編集する。

圧縮ライブラリのリンクオプションが-lzになっているので、-lzdllに書き換える（２ヶ所）。
pthreadライブラリのリンクオプションが-lpthreadになっているので、-lpthreadGC2に書き換える（２ヶ所）。

(3)Makefile.inも若干書き換える。

MinGW用ターゲットのリンクコマンドラインに-lzが書いてあるので-lzdllに書き換え。
Windows用のパッケージ作成ターゲットで、*.dllや*.hをCygwin環境でのパスで参照しているので、必要に応じて書き換え。
Java バインディングやRuby バインディングが不要なら、パッケージ作成ターゲットの記述からはずす。

(4)ビルド
上の(2),(3)をパッチファイルにまとめるとこんな感じ→HyperEstraierをMinGWで再ビルドするときの変更点

cd hyperestraier-1.4.13/
patch -p1 < ../hyperestraier-1.4.13-build-on-mingw.patch
./configure
make winpkg

これでWindows用のバイナリパッケージファイルhyperestraier-1.4.13-win32.zipが生成されます。

2008-01-20

しゅぎょうが足りない

読書ライトノベル

あと、

こちらも読み終わっていました。こちらは野尻ボードで見かけ、うーん、確かにゆるゆる感は気持ちがいいんですが...
まだまだ、私はSFものを名乗るには修行が足りなすぎるようです。
2巻の後のほう、タイムパラドックスものと気づくのも遅れました。出直してきます。