CEDEC2015のCoverityクイズをどう直すか

このエントリーをはてなブックマークに追加

CEDEC2015では、Coverity社のブースでバグのあるコードが掲示されていました。

バグがあるかどうかはCoverityが検出してくれるとして、じゃあどう解決すれば良いのか？を考えました。

コードを見る限り、

  • 配列の添え字と同じ値を入れたい
  • 処理は2つのクラスで共通化したい

という要求があると考えます。
(zに値を入れないが元コードもそうなのでよしとする)

また、簡単化のために与えられるデータは全て正しいと仮定します。
(実際は適切な箇所でエラーチェックが必要)

テンプレートでキャストを回避する

今回のバグは暗黙的キャストが行われ、かつキャストした状態でアドレス計算をするのが問題のため、
キャストされないように、テンプレートを使って両方のクラス用の関数を用意する事で回避する方法です。

#include <stdio.h>

class base_class {
public:
  base_class() { x = 0; y = 0; }

public:
  int x ;
  int y;
};

class derived_class : public base_class {
public:
  derived_class() { z = 0; }

public:
  int z;
};

template <typename T>
void calc_class_members(T b, int array_size)
{
  for(int i = 0; i < array_size; i++)
  {
    b[i].x = i;
    b[i].y = i;
  }
}

int update_class_members(
  derived_class * class_array,
  int num_array )
{
  if( class_array == NULL)
    return -1;

  calc_class_members(class_array, num_array);

  return 0;
}

アドレス計算を先にする

型が違う状態でアドレス計算しているのが問題なので、アドレス計算部分を外に出してしまう方法です。
関数ではなくクラスにメソッドとして持たせるとより良さそうです。

ただしこの場合、他の誰かが元コードのような処理を書くことを防げません。

void calc_class_member(base_class *b, int i)
{
  b->x = i;
  b->y = i;
}

int update_class_members(
  derived_class *class_array,
  int num_array )
{
  if( class_array == NULL)
    return -1;

  for(int i = 0; i < num_array; i++){
	calc_class_member(&class_array[i], i);
  }

  return 0;
}

設計から変える

意図的にバグを入れているコードに対して、全体を書き換えるのはナンセンスな気がしますが、
たぶんこういうことをする前提なら最も良い書き方はこうではないかと。

おおよそ上の通りですが、vectorの場合は中身の型が違うvectorにキャストされないため、
型が違う状態でアドレス計算が行われることはありません。

#include <vector>
#include <stdio.h>

class base_class {
public:
  base_class() { x = 0; y = 0; }

  void calc_class_member(int i)
  {
    x = i;
    y = i;
  }

public:
  int x ;
  int y;
};

class derived_class : public base_class {
public:
  derived_class() { z = 0; }

public:
  int z;
};


void update_class_members(std::vector<derived_class> &a)
{
  size_t size = a.size();
  for(size_t i = 0; i < size; ++i)
  {
    a.at(i).calc_class_member(static_cast<int>(i));
  }
}