Skip to content
This repository has been archived by the owner on Sep 7, 2021. It is now read-only.
This repository is currently being migrated. It's locked while the migration is in progress.

reflect.Value.Bytes() should only be called when reflect.Value.Kind()… #1346

Merged
merged 1 commit into from
Jul 5, 2019

Conversation

teejays
Copy link
Contributor

@teejays teejays commented Jul 5, 2019

Fix for issue #1345.

I noticed that in value2Interface function in session_convert.go, we are using reflect.Value.Bytes() function when the fieldType is reflect.Array OR reflect.Slice. However, go's reflect.Value.Bytes() only works when the value is of type reflect.Slice, and panics otherwise.

I would propose that change the if condition from:

if (k == reflect.Array || k == reflect.Slice) && (...) {
    // logic
}

to

if (k == reflect.Slice) && (...) {
    // logic
}

This PR fixes this.

@codecov-io
Copy link

codecov-io commented Jul 5, 2019

Codecov Report

Merging #1346 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1346   +/-   ##
=======================================
  Coverage   57.48%   57.48%           
=======================================
  Files          44       44           
  Lines        7833     7833           
=======================================
  Hits         4503     4503           
  Misses       2773     2773           
  Partials      557      557
Impacted Files Coverage Δ
session_convert.go 22.29% <100%> (ø) ⬆️
xorm.go 66.66% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4c80660...2484945. Read the comment docs.

@teejays
Copy link
Contributor Author

teejays commented Jul 5, 2019

Do I need to do something to pass the WIP check? :/

@lunny lunny added the kind/bug label Jul 5, 2019
@lunny lunny merged commit 9016f95 into go-xorm:master Jul 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants