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

More descriptive name for HandleEth() #30

Open
nickaxgit opened this issue Jul 15, 2024 · 7 comments
Open

More descriptive name for HandleEth() #30

nickaxgit opened this issue Jul 15, 2024 · 7 comments

Comments

@nickaxgit
Copy link

Brainstorm:-

func (ps *PortStack) handleEth(dst []byte) (n int, err error) {
//might be better named as getNextOutboundPacket
or fillNextOutboundPacket

@soypat
Copy link
Owner

soypat commented Jul 19, 2024

Note, the Put term comes from Go's encoding/binary package terminology which refers to writing data of length L into a buffer that needs to be at least of length L. See
More ideas

  • PutNextOutboundPacket
  • PutOutbound
  • PutNextPacket

@nickaxgit
Copy link
Author

nickaxgit commented Jul 19, 2024 via email

@soypat
Copy link
Owner

soypat commented Jul 20, 2024

So it occured to me that to better match with RecvEth maybe we could do PutOutboundEth, what do you think? Packet is pretty generic and Eth makes it clear it refers to link layer packets.

@nickaxgit
Copy link
Author

nickaxgit commented Jul 20, 2024 via email

@soypat
Copy link
Owner

soypat commented Jul 20, 2024

I agree the naming should be something between PutOutboundEth and PutNextOutboundEthPacket. I tend to err on the side of shortness since this will form part of a "famous" and fundamental interface: netif.InterfaceEthPoller.

This interface should be the first thing developers and users study when beginning working with any Go link-layer API. And I tend to disagree on the point of fundamental things needing be more explicit. In any case it being so fundamental should be more reason for the name to be short and memorable since it will be something outward facing and well known. We don't do Vector.AddVector(v Vector), in Go we prefer Vector.Add(v Vector). This is what gonum does, and that's a project I have huge respect for in terms of API design

I also feel giving it a long name would not be consistent with the naming we have given RecvEth. Even though RecvEth has a short name and omits the Next and Packet words in the name (even though they'd serve to describe it better) the purpose is quite clear- the Stack receives an Ethernet (packet! what else would it receive!). There is no question about whether it is the next or previous packet :)

@nickaxgit
Copy link
Author

nickaxgit commented Jul 20, 2024 via email

@soypat
Copy link
Owner

soypat commented Jul 20, 2024

Hey nick, I applied the changes in this commit: 74e7a6b

Also centralized the documentation at the handlers to avoid duplicating comments. Should be easily navigable in VSCode since the [iudphandler] bracket tagging allows for Ctrl+clicking in VSCode to easily go to the interface definition with the documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants