バグは誰のものか - エラー処理の責務分担

まかせてはいるけれども、たえず頭の中で気になっている。そこでときに報告を求め、問題がある場合には、適切な助言や指示をしていく。それが経営者のあるべき姿だと思います。これは言いかえますと“まかせてまかせず”ということになると思います。まかせてまかせずというのは、文字どおり“まかせた”のであって、決して放り出したのではないということです。

前回単体テストに関する話を書いたのでその続きで。

単体テストという言葉の定義はプロジェクトによって様々ですが、もっとも多いのは”関数”を単位とするものかと思います。関数単位のテストでは正常系のテスト(関数が適切に使われた時のテスト)以外に、異常値を与えられた時のテストも行われます。例えば以下の例。
const char*
month_name(int month)
{
  const char* name[] = {"Jan", "Feb", "Mar", "Apl", "May", "Jun",
                        "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"};

  if (month < 1 || month >= 12} 
    return NULL; /* invalid month */
  return name[month - 1];
}
月を文字列に変換する関数ですが、1〜12以外はエラーとしてNULL(0)を返しています。
このような関数の単体テストでは、1〜12の値以外に、エラー値として、0や-1, 13等も異常値としてテストすることになると思います。

このように、多くの関数では、引数として取りうる値や条件が限られている場合が多く、それを満たさない場合は、エラーを返すというのはよく見られることです。しかし、このようなエラー処理は、毎回必要なことでしょうか。

例えば、この関数が以下のような関数の内部で使われている場合を考えます。
int
print_date(int month, int day)
{
  if (month < 1 || month >= 12} 
    return -1;
  .....
  printf("%s-%d\n", month_name(month), day); 
}
先ほどと同じ範囲チェックがここでもされています。もし、month_name()がここでしか使われない、つまり、この関数の専用内部関数なのであれば、month_name()内の範囲チェックは冗長です。言い換えると、month_name()内のエラーチェックは内部のバグでもない限り発生することはありません。そもそも、print_date()では、month_name()でエラー、つまり、NULLがかえってくることを想定していないので、万が一NULLが返ってきたらNULL pointerアクセスでsegmentation fault等を引き起こします。

非常に単純な例ではありますが、このようなケースでは、month_name()でのエラー処理は不要で、必要なのはアサーションになります。つまり、以下のような形です。
const char*
month_name(int month)
{
  const char* name[] = {"Jan", "Feb", "Mar", "Apl", "May", "Jun",
                        "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"};
  assert(month > 0 && month < 12)
  return name[month - 1];
}
こう書くことで、この関数は前提条件として、引数に1から12しかとらないということが表現されます。範囲チェックをさぼっているのではなく「上位にまかせている」ということがこれではっきりします。アサーションであれば、処理されないようなエラーを返すコードもないので、そもそも単体テストも必要ありません。万が一、上位関数のロジックが間違っていれば、assertにひっかけて早期検出も可能です。
よく混同されがちですが、アサーションはエラー処理の一種ではないのです。

じゃあ、どうやって使い分けるの?という話になりますが、必ずしも一概には言いきれないものの、一般論として言えば、外部要因によって値が不正になるような場合(例えば、ライブラリの外部公開関数や、データが外部ファイルやネットワーク、ユーザー入力に由来するなど)には、エラー処理が必要になりますが、内部関数の場合は、大抵はエラー処理ではなく、アサーションが適当な場合が多くなります。

よく、エラー処理とアサーションが混同されているコードに以下のようなものがあります。
int
print_date(int month, int day)
{
  if (month < 1 || month >= 12) {
    assert(0);
    return -1;
  }
  .....
  printf("%s-%d\n", month_name(month), day); 
}
エラー処理の中に、assert(0);が書かれており、どっちつかずになってしまっています。但し、この関数が通常外部関数で使われているものであり、デバッグ用にエラー発生次第abortするという機能として、コンパイルオプション等で有効無効が設定できるようなアサーションなのであれば、それはデバッグを効率化するひとつの方法だと思います。

ここまでの例では、引数チェックの話を例にとりましたが、戻り値のチェックでも同じことが言えます。
  ....
  ercd = loc_mtx(MTX1); /* lock mutex */
  assert(ercd == E_OK);
  ....
mutexの取得失敗をassertで確認しています。通常、mutexの取得はパラメータエラーや、同一タスクでの二重取得等の内部ロジックの不具合でしか起こらないことが多く、ここでエラーを返しても上位関数ではどうすることもできないので、このような場合は、アサーションが適当です。(失敗するはずはないという表明になる)

もし、これを以下のように書こともできますが、結果としての処理は同じでも、コードとしては通りもしないエラー処理を書くことになり、C0やC1カバレッジを必要とするような場合には、困ることになります (通らないはずのコードなのだからパスを通しようがない)。
  ....
  ercd = loc_mtx(MTX1); /* lock mutex */
  if (ercd != E_OK) {
    printf("loc_mtx failed : %d\n", ercd);
    abort();
    ....

非常に単純な話をくどくどと書いてしまいましたが、各モジュールや関数の責務をはっきりさせ、効率的にテスト、デバッグする上で結構重要な観点だと思います。
B00JEYPPOE
【関連記事】
【関連書籍】

コメント

非公開コメント

本のおすすめ

4873115655

4274065979

4822236862

4274068579

4822255131

B00SIM19YS


プロフィール

  • Author:proger
  • 組み込み関係で仕事してます

ブログ内検索