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

x/sys/unix: add consts, structs and ioctl calls from <mtd/mtd-user.h> #46063

Closed
lhl2617 opened this issue May 9, 2021 · 6 comments
Closed

x/sys/unix: add consts, structs and ioctl calls from <mtd/mtd-user.h> #46063

lhl2617 opened this issue May 9, 2021 · 6 comments
Labels
Milestone

Comments

@lhl2617
Copy link

@lhl2617 lhl2617 commented May 9, 2021

What did you expect to see?

It would be convenient to have MTD user space constants, utils and ioctl operations in the package.
An example use case is in Facebook's OpenBMC repo here.

The proposed addition entails:

  • Adding #include <mtd/mtd-user.h> in unix/mkerrors.sh with the sufficient regex changes
  • Adding #include <mtd/mtd-user.h> in unix/linux/types.go with the necessary bindings (C.struct_XXX for structs, constants for enum)
  • Adding the necessary ioctl functions in unix/ioctl.go for unique types.
@gopherbot gopherbot added this to the Proposal milestone May 9, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented May 9, 2021

Change https://golang.org/cl/318211 mentions this issue: x/sys/unix: add consts, structs and ioctl calls for <mtd/mtd-user.h>

@lhl2617
Copy link
Author

@lhl2617 lhl2617 commented May 9, 2021

According to @robpike in #14873:

My point is that I don't want to see ioctl get helpers to make it look nice. It really doesn't need int and uint and *Termio and *Whatever helpers. The caller can do the conversion. It may be uglier but it will actually be faster, and anyway ioctl is hideous and I prefer not to hide that fact.

If necessary I will back off the ioctl functions change in unix/ioctl.go and leave the conversions to the caller. Opinions?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented May 10, 2021

For better or for worse, I think that golang.org/x/sys/unix has clearly gone in a different direction since Rob's comment from five years ago.

@robpike
Copy link
Contributor

@robpike robpike commented May 10, 2021

Oh, worse. Yes, worse.

@lhl2617
Copy link
Author

@lhl2617 lhl2617 commented May 11, 2021

I've backed out the ioctl wrappers in
https://go-review.googlesource.com/c/sys/+/318211/

I'll be happy to maintain another package using these structs and constants--easier and better to not further pollute x/sys

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented May 11, 2021

This doesn't need to be a proposal. It's fine to add constants and types from system header files to x/sys/unix without writing a separate proposal for each set.

@ianlancetaylor ianlancetaylor changed the title proposal: x/sys/unix: add consts, structs and ioctl calls from <mtd/mtd-user.h> x/sys/unix: add consts, structs and ioctl calls from <mtd/mtd-user.h> May 11, 2021
@ianlancetaylor ianlancetaylor modified the milestones: Proposal, Backlog May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants