std.socket cleanup

[CyberShadow]

Take three. Previous pull request: #211, newsgroup discussion.

[JakobOvrum]

Why did you switch to uint16_t instead of ushort? ushort is also guaranteed to be 16 bits. Wouldn't this change just confuse D programmers who aren't familiar with the C way?

The constructor this(AddressFamily af, SocketType type, string protocolName) should be this(AddressFamily af, SocketType type, in char[] protocolName) by the looks of it.

The Service class has this problem too. And Protocol. And InternetHost. Ok it looks like pretty much the whole module uses string everywhere. I don't know if that's a holdover from the old D1 module or not, but it should be changed.

In serviceToPort, service == "" should be changed to service.empty.

[CyberShadow]

The constructor this(AddressFamily af, SocketType type, string protocolName) should be this(AddressFamily af, SocketType type, in char[] protocolName) by the looks of it.

This shouldn't break any code, right? What about other methods, such as Service.getServiceByName?

And why did you switch to uint16_t instead of ushort? ushort is also guaranteed to be 16 bits. Wouldn't this change just confuse D programmers who aren't familiar with the C way?

I wanted to be consistent, but I didn't think of the time about the impact on the documentation. I'll take care of it.

[JakobOvrum]

This shouldn't break any code, right? What about other methods, such as Service.getServiceByName?

servent.s_name is a char*, so to!string(serv.s_name) creates a copy. getservbyname takes a const char*, so const(char)[] is sufficient.

[CyberShadow]

I understand the intent of the change, but that doesn't answer my question. Your observation also applies to similar methods which accept string parameters, such as Service.getServiceByName, correct?

[JakobOvrum]

Yes, they should not ask for the strong immutable guarantee from the user when they don't use it. Service.getServiceByName (which is a deliciously redundant name) does not need the immutable guarantee on its parameters because they are not escaped. This applies to all instances of string parameters I have seen in this module so far.

[CyberShadow]

Done, thanks.

[andralex]

Guess this is ready for committing. Does anyone have issues with it?

[CyberShadow]

For the record, I noticed that a DMD unit test (runnable/testsocket.d) fails on the pull request auto-tester, but I can't reproduce the problem on my machine.

Edit: never mind, I see the problem. It's caused by fixing enforcement of SocketSet capacity.

[CyberShadow]

That test is invalid: a class destructor accesses a heap-allocated field, which is what causes the segfault. The Socket class already has a destructor which closes the socket. I'll send a pull request.

Edit: #410.

[braddr]

I want that test yanked from the dmd suite and moved to a standard phobos unittest. It's from the era where the dmd suite contained tests for the whole codebase. Doesn't need to be done as part of this pull request though.

[CyberShadow]

Does anyone know what that test is supposed to be testing? It's already catching all SocketExceptions, so I guess it's testing for something else? Bad codegen? A segfault? What's PERMUTE_ARGS?

[braddr]

It dates back to a very old bug: http://d.puremagic.com/issues/show_bug.cgi?id=268

Obviously a number of issues weren't well handled back then.

[CyberShadow]

Ping?