Skip to content

Commit

Permalink
Using constant maxFieldIndex to clean up bounds checking
Browse files Browse the repository at this point in the history
  • Loading branch information
pascallouisperez committed Feb 22, 2018
1 parent f9e3251 commit b50f52e
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 7 deletions.
4 changes: 2 additions & 2 deletions parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,8 @@ func (p *parser) parseField() (*Field, error) {
if err != nil {
return nil, err
}
index := 65536 + 1
if len(sIndex) <= len("65536") {
index := maxFieldIndex + 1
if len(sIndex) <= len(strconv.Itoa(maxFieldIndex)) {
index, err = strconv.Atoi(sIndex)
if err != nil {
// unexpected since sIndex should conform to pIndex
Expand Down
7 changes: 5 additions & 2 deletions tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,11 @@ package worksheets

import (
"fmt"
"math"
)

const maxFieldIndex = math.MaxUint16

type Definition struct {
name string
fieldsByName map[string]*Field
Expand All @@ -32,8 +35,8 @@ func (def *Definition) addField(field *Field) error {

if _, ok := def.fieldsByIndex[field.index]; ok {
return fmt.Errorf("%s.%s: index %d cannot be reused", def.name, field.name, field.index)
} else if field.index > 65536 {
return fmt.Errorf("%s.%s: index cannot be greater than 65536", def.name, field.name)
} else if field.index > maxFieldIndex {
return fmt.Errorf("%s.%s: index cannot be greater than %d", def.name, field.name, maxFieldIndex)
}
def.fieldsByIndex[field.index] = field

Expand Down
18 changes: 15 additions & 3 deletions worksheets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,18 @@ func (s *Zuite) TestExample() {
require.False(s.T(), isSet)
}

func (s *Zuite) TestNewDefinitionsGood() {
cases := []string{
`worksheet simple {
65535:index_at_max bool
}`,
}
for _, ex := range cases {
_, err := NewDefinitions(strings.NewReader(ex))
assert.NoError(s.T(), err, ex)
}
}

func (s *Zuite) TestNewDefinitionsErrors() {
cases := map[string]string{
// crap input
Expand All @@ -60,12 +72,12 @@ func (s *Zuite) TestNewDefinitionsErrors() {

// worksheet semantics
`worksheet simple {
65537:index_too_large bool
}`: `simple.index_too_large: index cannot be greater than 65536`,
65536:index_too_large bool
}`: `simple.index_too_large: index cannot be greater than 65535`,

`worksheet simple {
9999999999999999999999999999999999999999999999999:index_too_large bool
}`: `simple.index_too_large: index cannot be greater than 65536`,
}`: `simple.index_too_large: index cannot be greater than 65535`,

`worksheet simple {
0:no_can_do_with_zero bool
Expand Down

0 comments on commit b50f52e

Please sign in to comment.