fix(inet): Return port as number in getsockname

[georgeto]

The documentation of getsockname() states that the port is returned as a number, which is also what the similar getpeername function does.

Closes #391

[alerque]

Thanks for reporting and fixing this.

The fix looks right to me, does it pass the sniff test for you @siffiejoe?

I guess my only question would be is there any circumstance where this would be a breaking change for existing usage? In most places Lua is going to cast this anyway. Is there any use case where this is going to break? Perhaps people have written conditionals like if port == "25" then ... are we going to screw them up?

If so should we make this a major release bump? Or just change the documentation to say we return a string and leave it like it is?

[georgeto]

I guess my only question would be is there any circumstance where this would be a breaking change for existing usage?

Also when used as parameter in string.format().

[alerque]

Also when used as parameter in string.format().

As far as I know all supported versions of Lua will cast a number to a string when used with string.format():

local port = 80
print(string.format("example.com:%s", port))

Likewise string concatenation works

print("example.com:"..port)

So far I haven't come up with a likely situation that would break except equality comparison

return port == "80"

Unfortunately that kind of test is likely to exist in the wild making this a potential breaking change.

What would be the harm in changing the documentation to document that we return a string and just leaving it?

[siffiejoe]

The string.format snippet would fail in Lua 5.1. I think it is definitely a breaking change, but it is worth fixing nevertheless. The code change looks good to me.

[alerque]

The string.format snippet would fail in Lua 5.1.

Why do you say that?

$ lua5.1 -e 'port = 6; print(string.format("example.com:%s", port))'
example.com:6

I think it is definitely a breaking change, but it is worth fixing nevertheless. The code change looks good to me.

Fair enough, thanks for the review.

I'll probably hold of on merging this for a little bit until I sort out where we are at with other minor changes that maybe should be released before we send out a breaking one.

[siffiejoe]

The string.format snippet would fail in Lua 5.1.

Why do you say that?

$ lua5.1 -e 'port = 6; print(string.format("example.com:%s", port))'
example.com:6

My bad. I just remembered that Lua 5.1 uses luaL_checklstring instead of luaL_tolstring, but it only makes a difference for values with a __tostring metamethod, not for plain numbers.

[alerque]

Yes. I believe it also makes a difference for nil values, just not for numbers.