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

Proposal: simplified Select(..) method #7

Open
cmaglie opened this issue Nov 7, 2016 · 3 comments
Open

Proposal: simplified Select(..) method #7

cmaglie opened this issue Nov 7, 2016 · 3 comments

Comments

@cmaglie
Copy link

cmaglie commented Nov 7, 2016

I'm using this library in my serial library project here an extract:

r := &goselect.FDSet{}
r.Set(uintptr(port.handle))
r.Set(uintptr(port.closeSignal.ReadFD()))
e := &goselect.FDSet{}
e.Set(uintptr(port.handle))
e.Set(uintptr(port.closeSignal.ReadFD()))

max := port.closeSignal.ReadFD()
if port.handle > max {
    max = port.handle
}
if err = goselect.Select(max+1, r, nil, e, -1); err != nil {
    return 0, err
}

I'm wondering if it's possible to make a Select method that reduce the boilerplate code and handles all the boring stuff automatically (for example the very annoting max+1 bit).

A proposal for a possible API, to simplify the above use case, may be the following:

// NewSet creates an array that holds all the file descriptors
// and automatically converts them from int to uintptr if needed
fds := goselect.NewSet(port.handle, port.closeSignal.ReadFD())

// The same set can be used for read, write and error
// (max+1) is determined internally
res, err := goselect.Select(fds, nil, fds)
if err != nil {
    return err
}

// The result contains all the three result sets (read, write, errors) and each one
// can be queried
if res.IsReadable(port.handle) {
    ...do something...
}

Is this a proposal that can be considered?

Thanks!

@creack
Copy link
Owner

creack commented Nov 7, 2016

I think we should keep the Select itself as close as the syscall as possible, but a helper doing what you suggest would be welcome.

I might also consider renaming Select into SysSelect or something like that and use Select for the simplified version.

@cmaglie
Copy link
Author

cmaglie commented Nov 7, 2016

Ok, I'll give it a try and make a PR (unless you want to work on it 😉)

@creack
Copy link
Owner

creack commented Nov 7, 2016

I'd love to work on it, but time is scarce ^^ so be my guest! :)

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