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

Add SO_LINGER option #42

Closed
sateffen opened this issue Jul 5, 2016 · 10 comments
Closed

Add SO_LINGER option #42

sateffen opened this issue Jul 5, 2016 · 10 comments

Comments

@sateffen
Copy link
Contributor

sateffen commented Jul 5, 2016

Hey,

because I didn't submit a pull-request (hopefully yet, up till now I'm missing time) I want to add this issue as reminder. Maybe someone else wants to implement this, but I think it has to be implemented at some point (and yes, you can implement it as well, I don't mind)

According to linux and windows documentation, this is just passing a struct looking like

struct linger {
    int l_onoff;    /* linger active */
    int l_linger;   /* how many seconds to linger for */
};

as optval for SO_LINGER. This should work cross plattform as far as I know.

I think the API should take a duration and convert that in an on-off version of the struct. What do you guys think?

@sateffen sateffen mentioned this issue Jul 5, 2016
@alexcrichton
Copy link
Contributor

Yeah sounds like this should be:

fn linger(&self) -> io::Result<Option<Duration>>;
fn set_linger(&self, dur: Option<Duration>) -> io::Result<()>;

Other than that looks good to me!

@sateffen
Copy link
Contributor Author

sateffen commented Jul 6, 2016

That API looks like the one I imagined, nice :)

Just one question: Why giving an option, and not the duration itself? A duration of 0 tells me to turn it of as well, just like giving None. Is there anything special or nice about this, that I don't see?

@alexcrichton
Copy link
Contributor

We tend to try to distinguish with types rather than sentinel values, but I'm not sure if Some(Duration::new(0, 0)) is actually possible

@sateffen
Copy link
Contributor Author

sateffen commented Jul 9, 2016

Just tried the Duration::new(0, 0) with rust 1.10, and it worked. Otherwise this would be a bug in the std lib in my opinion. Durations may reach the zero^^

@sateffen
Copy link
Contributor Author

sateffen commented Jul 9, 2016

I've added a pull-request for the SO_LINGER option. I tested this on windows, and it works, even though it tells me lingering is off by default.

This option doesn't exist for udp sockets as far as I read, so it's only implemented for the tcp parts.

@alexcrichton
Copy link
Contributor

Awesome, thanks!

@sateffen
Copy link
Contributor Author

sateffen commented Jul 9, 2016

Hmm, I just realized, that the libc binding doesn't expose the linger struct. I filed a bug about it. The winapi crate exposes this struct, so on windows this works flawlessly.

Maybe I'll find some more time tomorrow to open up a pull-request to the libc bindings, or someone else has some time to fix this issue. Afterwards this PR would work.

Or do you prefer an own struct for this? This would solve the problem immediatly, without updating the dependencies, but introduces some inconvenience, and could lead to problems if it gets added later on.

Any opinions?

@alexcrichton
Copy link
Contributor

Yeah let's try to add this to libc if we can, no reason it's not there other than someone just hasn't done it yet!

@sateffen
Copy link
Contributor Author

The pullrequest is merged, and in my opinion this issue is resolved. Is there anything missing, that needs to be done?

@alexcrichton
Copy link
Contributor

Aha yes, indeed!

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