Keyboard shortcuts

Press ← or → to navigate between chapters

Press S or / to search in the book

Press ? to show this help

Press Esc to hide this help

[インデックス 18901] ファイルの概要

このコミットは、Go言語の標準ライブラリであるbytesパッケージとstringsパッケージ内のReader型におけるUnreadRuneメソッドのバグ修正に関するものです。具体的には、UnreadRuneが直前の操作がReadRuneでなかった場合にエラーを返すように修正されています。これにより、UnreadRuneの予期せぬ動作を防ぎ、APIの契約をより厳密に遵守するようになります。

コミット

commit a509026ff0010dc29068983bd748c1360e692602
Author: Rui Ueyama <ruiu@google.com>
Date:   Wed Mar 19 09:00:58 2014 -0700

    strings, bytes: fix Reader.UnreadRune
    
    UnreadRune should return an error if previous operation is not
    ReadRune.
    
    Fixes #7579.
    
    LGTM=bradfitz
    R=golang-codereviews, bradfitz
    CC=golang-codereviews
    https://golang.org/cl/77580046

GitHub上でのコミットページへのリンク

https://github.com/golang/go/commit/a509026ff0010dc29068983bd748c1360e692602

元コミット内容

strings, bytes: fix Reader.UnreadRune

UnreadRuneは、直前の操作がReadRuneでなかった場合にエラーを返すようにすべきである。

Fixes #7579.

変更の背景

Go言語のioパッケージには、データの読み書きを行うための様々なインターフェースが定義されています。その中でも、io.Readerはバイト列を読み込むための基本的なインターフェースであり、io.ByteReaderやio.RuneReaderはそれぞれ1バイトや1Unicodeコードポイント（rune）を読み込むための拡張インターフェースです。

bytes.Readerとstrings.Readerは、それぞれバイトスライスと文字列をデータソースとして、これらのioインターフェースを実装しています。これらのリーダーには、読み込んだデータを「元に戻す」ためのUnreadByteやUnreadRuneといったメソッドが提供されています。

しかし、UnreadRuneメソッドのこれまでの実装では、直前の操作がReadRuneでなかった場合でもエラーを返さずに、不正な状態になる可能性がありました。これは、UnreadRuneが「直前に読み込んだruneをストリームに戻す」という操作であるため、直前にReadRune以外の操作（例えばRead、ReadByte、Seekなど）が行われていた場合、どのruneを戻すべきか不明確になり、予期せぬ動作やデータ破損につながる恐れがありました。

この問題は、GoのIssue #7579として報告されており、このコミットはその問題を解決するために行われました。UnreadRuneの正しいセマンティクスは、直前の操作がReadRuneであった場合にのみ成功し、それ以外の場合はエラーを返すことです。これにより、リーダーの状態の一貫性が保たれ、より堅牢なコードの記述が可能になります。

前提知識の解説

このコミットを理解するためには、以下のGo言語の概念と標準ライブラリの知識が必要です。

1. ioパッケージとリーダーインターフェース

Go言語のioパッケージは、I/Oプリミティブを提供します。

  • io.Reader: Read(p []byte) (n int, err error)メソッドを持つインターフェースで、バイトスライスpにデータを読み込みます。
  • io.ByteReader: ReadByte() (byte, error)メソッドを持つインターフェースで、1バイトを読み込みます。
  • io.RuneReader: ReadRune() (r rune, size int, err error)メソッドを持つインターフェースで、1つのUnicodeコードポイント（rune）とそのバイトサイズを読み込みます。
  • io.Seeker: Seek(offset int64, whence int) (int64, error)メソッドを持つインターフェースで、リーダーの読み取り位置を変更します。

2. bytes.Readerとstrings.Reader

  • bytes.Reader: []byte（バイトスライス）を読み取り元とするリーダーです。io.Reader, io.ByteReader, io.RuneReader, io.Seekerなどを実装しています。
  • strings.Reader: string（文字列）を読み取り元とするリーダーです。bytes.Readerと同様に、io.Reader, io.ByteReader, io.RuneReader, io.Seekerなどを実装しています。

これらのリーダーは、内部に現在の読み取り位置を示すインデックス（r.i）と、直前にReadRuneで読み込んだruneの開始位置を記憶するためのフィールド（r.prevRune）を持っています。

3. UnreadRuneメソッドのセマンティクス

UnreadRuneメソッドは、io.RuneReaderインターフェースを実装する型が提供する可能性のあるメソッドで、直前にReadRuneによって読み込まれたruneをストリームに「戻す」ことを目的としています。これにより、次にReadRuneが呼び出されたときに、同じruneが再度読み込まれるようになります。

重要なのは、UnreadRuneは直前の操作がReadRuneであった場合にのみ有効であるという点です。もし直前の操作がReadRune以外であった場合、UnreadRuneはどのruneを戻すべきかを知ることができません。このような状況でUnreadRuneが呼び出された場合、エラーを返すのが正しい振る舞いです。これは、io.Readerや関連するインターフェースの設計原則に基づいています。

4. r.prevRuneフィールド

bytes.Readerとstrings.Readerの内部には、prevRuneというフィールドが存在します。このフィールドは、ReadRuneが呼び出された際に、読み込んだruneの開始インデックスを記憶するために使用されます。UnreadRuneはこのprevRuneの値を利用して、読み取り位置を元に戻します。

このコミットの修正の核心は、ReadRune以外の操作が行われた際に、このprevRuneフィールドを無効な値（通常は-1）にリセットすることです。これにより、UnreadRuneが呼び出されたときにprevRuneが有効な値でなければエラーを返すというロジックが正しく機能するようになります。

技術的詳細

このコミットの技術的な詳細は、bytes.Readerとstrings.Readerの各読み取りメソッドにおけるr.prevRuneフィールドの管理方法の変更に集約されます。

GoのioパッケージにおけるUnreadRuneの一般的なセマンティクスは、直前の操作がReadRuneであった場合にのみ成功するというものです。それ以外の操作（Read、ReadAt、ReadByte、Seekなど）がReadRuneの後に実行された場合、UnreadRuneはエラーを返すべきです。

このコミット以前のbytes.Readerとstrings.Readerの実装では、ReadRune以外のメソッドが呼び出された際にr.prevRuneが適切にリセットされていませんでした。そのため、例えばReadRuneの後にReadByteが呼び出された場合でも、r.prevRuneにはReadRuneが読み込んだruneの開始位置が残ったままでした。この状態でUnreadRuneを呼び出すと、UnreadRuneはr.prevRuneが有効であると判断し、誤って読み取り位置を戻してしまう可能性がありました。

このコミットでは、この問題を解決するために、ReadRune以外のすべての読み取り操作（Read、ReadAt、ReadByte）および位置変更操作（Seek）の開始時に、r.prevRuneを無効な値である-1に設定するように変更されました。

具体的には、以下のメソッドの冒頭にr.prevRune = -1が追加されています。

  • func (r *Reader) Read(b []byte) (n int, err error)
  • func (r *Reader) ReadAt(b []byte, off int64) (n int, err error)
  • func (r *Reader) ReadByte() (b byte, err error)
  • func (r *Reader) Seek(offset int64, whence int) (int64, error)

また、ReadRuneメソッドのio.EOFを返すパス（つまり、読み取り対象のデータがもうない場合）でもr.prevRune = -1が設定されるようになりました。これは、ReadRuneがエラーを返した場合も、その後のUnreadRuneが成功しないようにするためです。

この変更により、r.prevRuneは常に直前の操作がReadRuneであった場合にのみ有効な値を持つようになります。UnreadRuneメソッドは、r.prevRuneが-1である場合にエラーを返すロジックを既に持っているため、この修正によってUnreadRuneの動作が期待通りになります。

さらに、この修正の正しさを検証するために、bytes/reader_test.goとstrings/strings_test.goに新しいテストケースTestUnreadRuneErrorが追加されました。このテストは、ReadRuneの後にRead、ReadAt、ReadByte、UnreadRune（二重呼び出し）、Seek、WriteToといった操作を行い、その後にUnreadRuneを呼び出した場合にエラーが返されることを確認します。これにより、UnreadRuneのセマンティクスが正しく実装されたことが保証されます。

コアとなるコードの変更箇所

このコミットにおけるコアとなるコードの変更は、src/pkg/bytes/reader.goとsrc/pkg/strings/reader.go内の複数のメソッドにr.prevRune = -1の行が追加された点です。

src/pkg/bytes/reader.go

--- a/src/pkg/bytes/reader.go
+++ b/src/pkg/bytes/reader.go
@@ -30,6 +30,7 @@ func (r *Reader) Len() int {
 }
 
 func (r *Reader) Read(b []byte) (n int, err error) {
+	r.prevRune = -1 // 追加
 	if len(b) == 0 {
 		return 0, nil
 	}
@@ -38,11 +39,11 @@ func (r *Reader) Read(b []byte) (n int, err error) {
 	}
 	n = copy(b, r.s[r.i:])
 	r.i += n
-	r.prevRune = -1 // 削除
 	return
 }
 
 func (r *Reader) ReadAt(b []byte, off int64) (n int, err error) {
+	r.prevRune = -1 // 追加
 	if off < 0 {
 		return 0, errors.New("bytes: invalid offset")
 	}
@@ -57,12 +58,12 @@ func (r *Reader) ReadAt(b []byte, off int64) (n int, err error) {
 }
 
 func (r *Reader) ReadByte() (b byte, err error) {
+	r.prevRune = -1 // 追加
 	if r.i >= len(r.s) {
 		return 0, io.EOF
 	}
 	b = r.s[r.i]
 	r.i++
-	r.prevRune = -1 // 削除
 	return
 }
 
@@ -77,6 +78,7 @@ func (r *Reader) UnreadByte() error {
 
 func (r *Reader) ReadRune() (ch rune, size int, err error) {
 	if r.i >= len(r.s) {
+		r.prevRune = -1 // 追加
 		return 0, 0, io.EOF
 	}
 	r.prevRune = r.i
@@ -100,6 +102,7 @@ func (r *Reader) UnreadRune() error {
 
 // Seek implements the io.Seeker interface.
 func (r *Reader) Seek(offset int64, whence int) (int64, error) {
+	r.prevRune = -1 // 追加
 	var abs int64
 	switch whence {
 	case 0:

src/pkg/strings/reader.go

同様の変更がstrings/reader.goにも適用されています。

--- a/src/pkg/strings/reader.go
+++ b/src/pkg/strings/reader.go
@@ -29,6 +29,7 @@ func (r *Reader) Len() int {
 }
 
 func (r *Reader) Read(b []byte) (n int, err error) {
+	r.prevRune = -1 // 追加
 	if len(b) == 0 {
 		return 0, nil
 	}
@@ -37,11 +38,11 @@ func (r *Reader) Read(b []byte) (n int, err error) {
 	}
 	n = copy(b, r.s[r.i:])
 	r.i += n
-	r.prevRune = -1 // 削除
 	return
 }
 
 func (r *Reader) ReadAt(b []byte, off int64) (n int int, err error) {
+	r.prevRune = -1 // 追加
 	if off < 0 {
 		return 0, errors.New("strings: invalid offset")
 	}
@@ -56,12 +57,12 @@ func (r *Reader) ReadAt(b []byte, off int64) (n int, err error) {
 }
 
 func (r *Reader) ReadByte() (b byte, err error) {
+	r.prevRune = -1 // 追加
 	if r.i >= len(r.s) {
 		return 0, io.EOF
 	}
 	b = r.s[r.i]
 	r.i++
-	r.prevRune = -1 // 削除
 	return
 }
 
@@ -76,6 +77,7 @@ func (r *Reader) UnreadByte() error {
 
 func (r *Reader) ReadRune() (ch rune, size int, err error) {
 	if r.i >= len(r.s) {
+		r.prevRune = -1 // 追加
 		return 0, 0, io.EOF
 	}
 	r.prevRune = r.i
@@ -99,6 +101,7 @@ func (r *Reader) UnreadRune() error {
 
 // Seek implements the io.Seeker interface.
 func (r *Reader) Seek(offset int64, whence int) (int64, error) {
+	r.prevRune = -1 // 追加
 	var abs int64
 	switch whence {
 	case 0:

テストファイルの追加

src/pkg/bytes/reader_test.goとsrc/pkg/strings/strings_test.goに、TestUnreadRuneErrorという新しいテスト関数が追加されました。このテストは、ReadRuneの後にRead、ReadAt、ReadByte、UnreadRune（二重呼び出し）、Seek、WriteToといったUnreadRuneの前提条件を壊す可能性のある操作を実行し、その後にUnreadRuneを呼び出した場合にエラーが返されることを検証します。

// bytes/reader_test.go および strings/strings_test.go に追加されたテスト
var UnreadRuneErrorTests = []struct {
	name string
	f    func(*Reader)
}{
	{"Read", func(r *Reader) { r.Read([]byte{}) }},
	{"ReadAt", func(r *Reader) { r.ReadAt([]byte{}, 0) }},
	{"ReadByte", func(r *Reader) { r.ReadByte() }},
	{"UnreadRune", func(r *Reader) { r.UnreadRune() }}, // 二重UnreadRuneのテスト
	{"Seek", func(r *Reader) { r.Seek(0, 1) }},
	{"WriteTo", func(r *Reader) { r.WriteTo(&Buffer{}) }}, // bytes.Buffer または bytes.Buffer{}
}

func TestUnreadRuneError(t *testing.T) {
	for _, tt := range UnreadRuneErrorTests {
		reader := NewReader([]byte("0123456789")) // stringsの場合は NewReader("0123456789")
		if _, _, err := reader.ReadRune(); err != nil {
			// should not happen
			t.Fatal(err)
		}
		tt.f(reader) // ReadRune以外の操作を実行
		err := reader.UnreadRune()
		if err == nil {
			t.Errorf("Unreading after %s: expected error", tt.name)
		}
	}
}

コアとなるコードの解説

このコミットの核心は、Reader構造体のprevRuneフィールドの厳密な管理にあります。

bytes.Readerとstrings.Readerは、内部に以下のフィールドを持っています。

  • s []byte または s string: 読み取り元のデータ。
  • i int64: 現在の読み取り位置（インデックス）。
  • prevRune int: 直前にReadRuneで読み込んだruneの開始インデックス。ReadRune以外の操作が行われた場合や、UnreadRuneが呼び出された場合は-1にリセットされます。

UnreadRuneメソッドは、prevRuneが-1でない場合にのみ、iをprevRuneの値に戻し、prevRuneを-1にリセットします。もしprevRuneが-1であれば、UnreadRuneはエラー（ErrInvalidUnread）を返します。

このコミット以前は、Read、ReadAt、ReadByte、Seekといったメソッドが呼び出されても、prevRuneはリセットされませんでした。そのため、ReadRuneの後にこれらのメソッドが呼び出されても、prevRuneには以前のReadRuneの開始位置が残ったままでした。この状態でUnreadRuneを呼び出すと、prevRuneが-1ではないため、UnreadRuneは成功してしまい、誤った位置にリーダーのインデックスを戻してしまう可能性がありました。これは、UnreadRuneが「直前のReadRune操作を元に戻す」というセマンティクスに反します。

今回の修正では、Read、ReadAt、ReadByte、Seekの各メソッドの冒頭でr.prevRune = -1と明示的に設定することで、これらの操作が行われた直後にはprevRuneが必ず無効な状態になるようにしました。これにより、これらの操作の後にUnreadRuneが呼び出された場合、prevRuneが-1であるため、UnreadRuneは正しくエラーを返すようになります。

また、ReadRuneがio.EOFを返す場合（つまり、読み取るデータがない場合）にもr.prevRune = -1を設定するようになりました。これは、ReadRuneがエラーを返した後にUnreadRuneが呼び出されても、それが成功しないようにするためです。

この変更は、bytes.Readerとstrings.ReaderのUnreadRuneメソッドの堅牢性を高め、APIの契約をより厳密に遵守させるための重要な修正です。これにより、これらのリーダーを使用するコードがより予測可能で安全になります。

関連リンク

参考にした情報源リンク