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

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
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@lhl2617
Copy link

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

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

@lhl2617 lhl2617 mentioned this issue May 9, 2021
Closed
@lhl2617
Copy link
Author

lhl2617 commented May 9, 2021 •
edited

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

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 commented May 10, 2021

Oh, worse. Yes, worse.

@lhl2617
Copy link
Author

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

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 added NeedsFix The path to resolution is known, but the work has not been done. and removed Proposal labels May 11, 2021
@ianlancetaylor ianlancetaylor modified the milestones: Proposal, Backlog May 11, 2021
@lhl2617 lhl2617 mentioned this issue May 29, 2021
Closed
@golang golang locked and limited conversation to collaborators May 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@ianlancetaylor @robpike @gopherbot @lhl2617