Socket.getaddrinfo handles special hosts '<any>' and '<broadcast>' #2115

Merged
merged 2 commits into from Jan 6, 2013

Conversation

Projects
None yet
2 participants
@sdaubert
Contributor

sdaubert commented Jan 4, 2013

As said on rubinius#2111, here is the new pull request to handle special hosts '' and '', without converting last item on returned array to a packed string.

This pull request also contains a commit to modify specs "Socket#gethostbyname returns broadcast address info for '<broadcast>'" and "Socket#gethostbyname returns broadcast address info for '<any>'" to only test the first item returned.

@dbussink

This comment has been minimized.

Show comment Hide comment
@dbussink

dbussink Jan 5, 2013

Member

Ok, looks good! One thing though, could you improve the layout of the commit messages? For example https://github.com/sdaubert/rubinius/commit/b24e1aa261bf6a660efa923b061fc2549ed89d08 has a long first line that overflows and the line breaks in the lines further seem weird too. In general the recommendation is to use up to 72 char lines.

Usually how it looks on github can be a good guideline for the messages. For a multiple line commit message, also use a newline after the first line and multiple paragraphs for really long texts.

Member

dbussink commented Jan 5, 2013

Ok, looks good! One thing though, could you improve the layout of the commit messages? For example https://github.com/sdaubert/rubinius/commit/b24e1aa261bf6a660efa923b061fc2549ed89d08 has a long first line that overflows and the line breaks in the lines further seem weird too. In general the recommendation is to use up to 72 char lines.

Usually how it looks on github can be a good guideline for the messages. For a multiple line commit message, also use a newline after the first line and multiple paragraphs for really long texts.

sdaubert added some commits Jan 4, 2013

Modify library/socket/socket/gethostbyname specs.
For specs "Socket#gethostbyname returns broadcast address info for
'<broadcast>'" and "Socket#gethostbyname returns broadcast address
info for '<any>'", only the first item (address) of returned array
is tested.

Later, some specs shall be added to verify that latest item of returned array contains
address as a packed string.
Modify Socket::Foreign.getaddrinfo to handle special host strings.
Modify Socket::Foreign.getaddrinfo to handle special host strings
'<any>' and '<broadcast>'.

Modification made for 1.8, 1.9 and 2.0 versions.
@sdaubert

This comment has been minimized.

Show comment Hide comment
@sdaubert

sdaubert Jan 6, 2013

Contributor

OK, commit messages have been modified.

Contributor

sdaubert commented Jan 6, 2013

OK, commit messages have been modified.

dbussink added a commit that referenced this pull request Jan 6, 2013

Merge pull request #2115 from sdaubert/socket.getaddrinfo_any_broadcast
Socket.getaddrinfo handles special hosts '<any>' and '<broadcast>'

@dbussink dbussink merged commit dcf1bd9 into rubinius:master Jan 6, 2013

1 check passed

default The Travis build passed
Details
@dbussink

This comment has been minimized.

Show comment Hide comment
@dbussink

dbussink Jan 6, 2013

Member

Thnx!

Member

dbussink commented Jan 6, 2013

Thnx!

@sdaubert sdaubert deleted the sdaubert:socket.getaddrinfo_any_broadcast branch Jan 6, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment