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.

Sign up for GitHub

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

Jump to bottom

rpc: avoid use of cgo by hard-coding maxPathSize #27447

Merged
merged 3 commits into from
Jun 19, 2023
Merged

rpc: avoid use of cgo by hard-coding maxPathSize #27447

merged 3 commits into from
Jun 19, 2023

Conversation

zchee
Copy link
Contributor

@zchee zchee commented Jun 9, 2023

Avoid using cgo with go tool cgo -godefs.

Generated by:

$ docker container run --rm -it --entrypoint=sh -v $PWD:/go/src/github.com/ethereum/go-ethereum -w /go/src/github.com/ethereum/go-ethereum golang:bullseye -c 'make godef'

@zchee zchee requested review from fjl and holiman as code owners June 9, 2023 16:27
@zchee
Copy link
Contributor Author

zchee commented Jun 9, 2023

If you want to get values per GOOS, commit those files too.

@fjl
Copy link
Contributor

fjl commented Jun 9, 2023

Hmm, we already have a version of this file without cgo, constants_unix_nocgo.go

@zchee
Copy link
Contributor Author

zchee commented Jun 9, 2023 •
edited
Loading

@fjl But document said:

On Linux, sun_path is 108 bytes in size

However, at least bullseye distro, have different size (0x6e) .
I think this change will get more realstic value.

EDIT: I misunderstood but no need for cgo build tag switching.

@zchee
Copy link
Contributor Author

zchee commented Jun 9, 2023

ah, I'm misunderstood. Fix later

@zchee
Copy link
Contributor Author

zchee commented Jun 10, 2023 •
edited
Loading

@fjl PTAL.

@holiman
Copy link
Contributor

holiman commented Jun 13, 2023

Sorry, I don't understand. I am not familiar with godefs. What is the problem that you want to solve with this PR?

@fjl
Copy link
Contributor

fjl commented Jun 13, 2023 •
edited
Loading

This PR removes the dependency on cgo by generating the max_path_size constant using the godefs tool. We use this constant to validate the length of IPC file names.

If the dependency on cgo is a problem, we could also just remove the constants_unix.go file and use our existing non-cgo alternative for all platforms. It does not make sense to me to replace it with godefs because the value will be the same.

The impact of a wrong constant value will be very low because it is only used for printing a warning message.

@zchee zchee force-pushed the rpc/uncgo branch 2 times, most recently from 5ca575e to d375525 Compare June 16, 2023 07:17
@zchee
Copy link
Contributor Author

zchee commented Jun 16, 2023 •
edited
Loading

@fjl
Respect your comment, rename constants_unix.go to constants.go. PTAL

@zchee zchee changed the title rpc: avoid use cgo with 'go tool cgo -godefs' rpc: avoid use cgo with hardcoded maxPathSize Jun 16, 2023
@fjl fjl changed the title rpc: avoid use cgo with hardcoded maxPathSize rpc: avoid use of cgo by hard-coding maxPathSize Jun 16, 2023
@fjl fjl merged commit f0b5af7 into ethereum:master Jun 19, 2023
@fjl fjl added this to the 1.12.1 milestone Jun 19, 2023
@zchee zchee deleted the rpc/uncgo branch June 19, 2023 06:09
@zchee
Copy link
Contributor Author

zchee commented Jun 19, 2023

@fjl Thanks!

@0xcb9ff9 0xcb9ff9 mentioned this pull request Oct 30, 2023
Merged
github-merge-queue bot pushed a commit to dogechain-lab/dbsc that referenced this pull request Oct 31, 2023
@0xcb9ff9 @bnovil @ajsutton
rpc: replace rpc module (#42)
a7208a2 
### Description

replacing `rpc` module with the upstream version

### Changes

Focus PR: 

*  ethereum/go-ethereum#26681
*  ethereum/go-ethereum#27447

---------

Co-authored-by: Brandon Liu <[email protected]>
Co-authored-by: Adrian Sutton <[email protected]>
devopsbo3 pushed a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
@zchee @fjl @devopsbo3
rpc: avoid use of cgo by hard-coding maxPathSize (ethereum#27447)
63eb2e9 
Package rpc uses cgo to find the maximum UNIX domain socket path 
length. If exceeded, a warning is printed. This is the only use of cgo in this
package. It seems excessive to depend on cgo just for this warning, so
we now hard-code the usual limit for Linux instead.

---------

Co-authored-by: Felix Lange <[email protected]>
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
@devopsbo3
Revert "rpc: avoid use of cgo by hard-coding maxPathSize (ethereum#27447
3e40df2 
)"

This reverts commit 63eb2e9.
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
@devopsbo3
Revert "rpc: avoid use of cgo by hard-coding maxPathSize (ethereum#27447
c4dcfc6 
)"

This reverts commit 63eb2e9.
alienc0der added a commit to alienc0der/go-zenon that referenced this pull request Mar 4, 2024
@alienc0der
rpc: avoid use of cgo by hard-coding maxPathSize
d83e297 
Implemented ethereum/go-ethereum#27447
@alienc0der alienc0der mentioned this pull request Mar 4, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@holiman holiman holiman left review comments

@fjl fjl fjl approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.12.1
Development

Successfully merging this pull request may close these issues.
3 participants
@zchee @fjl @holiman