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

Separate all parsing functions (or make public) #478

Open
4 tasks
Dentrax opened this issue Nov 4, 2022 · 1 comment
Open
4 tasks

Separate all parsing functions (or make public) #478

Dentrax opened this issue Nov 4, 2022 · 1 comment

Comments

@Dentrax
Copy link
Contributor

Dentrax commented Nov 4, 2022

Motivation

As procfs already using parseX(io.Reader) functions for parsing proc filesystem files, some functions also do syscall to read the target file' content. The main pitfall of this is that procfs enforce us to use this package only on Linux. For the people who want to make some stuff using this package on other OS environments, it's slightly difficult.

Proposal

  • Separate core parsing functions: (i.e. os.Open and parsing logic should not be allowed in same function)
  • These functions should NOT do any syscall (or depend on)
  • All functions should accept an input arg: io.Reader or []byte
  • Returned data types should also be exported:

Example

  1. Make all output types exported:
- func parseFoo() (foo, error)
+ func parseFoo() (Foo, error)
  1. Get rid of the receivers, if possible:
- func (fs FS) parseFoo() (Foo, error)
+ func parseFoo() (Foo, error)
  1. Convert all parsing functions to public:
- func parseFoo() (Foo, error)
+ func ParseFoo() (Foo, error)
  • Some parser funcs might be depended on a receiver such as FS. If received is really required during the parsing stage, keep it as-is for now

1. If we about to keep those as private

Anyone who want to just access parser functions, have to use go:linkname directive as like the following:

//go:linkname parseLine github.com/prometheus/procfs.NetDev.parseLine
func parseLine(a procfs.NetDev, b string) (*procfs.NetDevLine, error)

2. If we about to keep those as Public

  • No need to use go:linkname directive
  • Better control over all functions

3. Depend on a receiver or some function that do syscalls

We simply have to do CTRL+C/CTRL+V the actual logic. Which is decreasing the overall UX of the package.

Tracking (Merge Status)

Waiting your thoughts! I can make this issue as a tracking issue, if it does make sense in overall!

@discordianfish
Copy link
Member

This sounds reasonable to me. @SuperQ wdyt?

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