Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: upgrade to golangci-lint v1.39.0 #22696

Merged
merged 4 commits into from
Apr 27, 2021
Merged

Conversation

fjl
Copy link
Contributor

@fjl fjl commented Apr 19, 2021

This updates the lint tool and fixes the issues reported by the new tool version.

WARN [runner/nolint] Found unknown linters in //nolint directives: composites 
eth/catalyst/api_test.go:71:13: composites: `github.com/ethereum/go-ethereum/params.ChainConfig` composite literal uses unkeyed fields (govet)
	config := &params.ChainConfig{big.NewInt(1337), big.NewInt(0), nil, false, big.NewInt(0), common.Hash{}, big.NewInt(0), big.NewInt(0), big.NewInt(0), big.NewInt(0), big.NewInt(0), big.NewInt(0), big.NewInt(0), big.NewInt(0), nil, nil, big.NewInt(0), new(params.EthashConfig), nil}
	           ^
consensus/ethash/ethash.go:116:12: unsafeptr: possible misuse of reflect.SliceHeader (govet)
	header := *(*reflect.SliceHeader)(unsafe.Pointer(&mem))
	          ^
consensus/ethash/ethash.go:120:42: unsafeptr: possible misuse of reflect.SliceHeader (govet)
	return mem, *(*[]uint32)(unsafe.Pointer(&header)), nil
	                                        ^

header.Data = (*reflect.SliceHeader)(unsafe.Pointer(&mem)).Data
header.Cap = len(mem) / 4
header.Len = header.Cap
return mem, view, nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other similar conversions seem a bit different. Perhaps lets do this part the same way?

	var cache []byte
	cacheHdr := (*reflect.SliceHeader)(unsafe.Pointer(&cache))
	dstHdr := (*reflect.SliceHeader)(unsafe.Pointer(&dest))
	cacheHdr.Data = dstHdr.Data
	cacheHdr.Len = dstHdr.Len * 4
	cacheHdr.Cap = dstHdr.Cap * 4

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is no real difference between what you posted and what's in the diff.
In the []uint32 to []byte conversion in algorithm.go, it accesses the length of the
slice header instead of calling len. But they give the same result, and I prefer using len.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the difference is that the alternative uses dstHdr to retrieve the Len and Cap (no idea where these come from), vs you assume these will have certain values based on mem. I'm almost certain your code is fine since we never append to these constructed slices, but unsure what would hapen if we did.

@karalabe karalabe added this to the 1.10.3 milestone Apr 27, 2021
@karalabe karalabe merged commit a3f0da1 into ethereum:master Apr 27, 2021
atif-konasl pushed a commit to frozeman/pandora-execution-engine that referenced this pull request Oct 15, 2021
* build: upgrade to golangci-lint v1.39.0

* consensus/ethash: fix go vet warning regarding reflect.SliceHeader

* eth/catalyst: fix lint issue

* consensus/ethash: fix bug in memoryMapFile
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants