new MetaMessage() always throws an NegativeArraySizeException

trrk commented 9 years ago

new MetaMessage() を呼ぶと必ずNegativeArraySizeExceptionを投げられます。

new MetaMessage()の中でthis(defaultMessage)を呼んでいますが、その引数のサイズが2なので、その中のif(data.length<3)の文に引っかかっていました。

MIDIファイルを読み込ませるときにも発生し、Sequenceが作られない事態になっています。修正をお願いします。

kshoji commented 9 years ago

ご報告ありがとうごさいます。 data.lengthをチェックする所で3と比較していたのですが、これは2が正しいはずなので修正してみました。 developブランチに反映しています。確認していただいて、問題なければクローズします。

https://github.com/kshoji/javax.sound.midi-for-Android/tree/develop

trrk commented 9 years ago

ありがとうございます。エラーは起きなかったのですが、今改めて考えてみると、別の問題が発生してしまうと思います。

MetaMessageであるがゆえに、this.dataの構造はこのようになっていると思います。 [0xFF,{type},{メイン部の長さ・1バイト以上},{メイン部・ここはない場合もある}] ※defaultMessageは[0xFF,0]でこの構造でないですが。

dataLengthは、this.dataの{メイン部}の長さを格納していると思います。

{メイン部}以外の部分は必ず3バイト以上であるため、43行目の変更は元のdataLength = data.length - 3のままでよかったのではないでしょうか。同じ理由でif文の条件もdata.length < 3(else節をdata.length>=3の時に通る) に戻す。 (そのあとのwhile文は、{メイン部の長さ}が2バイト以上の場合に対応するためのものだと思います。)

こうすると結局、変更前の状態になってしまうので、・defaultMessageを変える・引数がdefaultMessageの時は無視 ( clone()を呼ばれた時のため、equalsで比較 ) 引数の長さが3未満なのはdefaultMessageだった場合しかあり得ないので、・そもそもdata.length<3で例外を投げるのをやめるといった修正がいいと思います。 ※自分が前やった、super(defaultMessage)を呼ぶという方法は、clone()を呼んだ場合にエラーが出てしまうのでダメでしたね。

51~54行目のdataLength<0もあり得ないので、いらないかなと思います。また、clone()に追加されたsuper.clone()についても多分不要だと思うのですがどうでしょうか。

例外を投げるのをやめると最終的なMetaMessage(byte[])のコードはこうです。

protected MetaMessage(@NonNull byte[] data) {
    super(data);
    if(data.length>=3){
        // check length
            dataLength = data.length - 3;
            int pos = 2;
            while (pos < data.length && (data[pos] & 0x80) != 0) {
                dataLength--;
                pos++;
            }
    }
}

長くなってしまいましたが、よろしくお願いします。

kshoji commented 9 years ago

返信遅くなってしまいました…

このIssueについては、結局のところ、defaultMessageがMetaMessageの仕様に合っていないのが問題だという気がします。

defaultMessageを3バイトのものに変える({ META, 0, 0 })
コンストラクタは以前のものに戻す

という方針で修正してみました(↑のコミット)。これでおそらく矛盾はしないと思います。

trrk commented 9 years ago

エラーも起きませんし、大丈夫だと思います。ただ、 if (dataLength < 0) は通ることがないような気がしますが…

修正ありがとうございました。

kshoji / javax.sound.midi-for-Android

new MetaMessage() always throws an NegativeArraySizeException #3