[erlang-patches] Montavista system call error handling patch

[Steve Vinoski]

2010/5/14 Björn Gustavsson <bgustavsson@REDACTED>:
> I agree with Steve's points.
>
> In the Erlang/OTP team, we usually don't use the
> term "broken platform". A platform may be broken
> according to some definition of "broken", but if a
> customer has chosen to use a certain platform
> and Erlang/OTP does not work well on it, the
> customer will blame us, not the platform. Pointing
> out to the customer that their chosen platform
> is "broken" will not do any good.
>
> The patch looks fine to me. (Except that the
> definition of SOCKET_ERROR should be removed
> since it is presumably no longer used.)

Done.

> Before accepting it, we will need to consider whether
> a successful return value could be interpreted as
> an error. That is, will any of the socket functions
> return a negative error that does not indicate an
> error?
>
> Looking at a few of the calls:
>
> The connect() call return 0 if successful, otherwise
> -1. It seems to me that if an implementation would
> return for instance -2 to mean success, it would be
> more horribly broken than Montavista Linux, so that
> should be safe. (Alternatively, in this case, it would
> be possible to test for 0, using another macro.)
>
> listen() has the same return values as connect().
>
> The man page for accept() says that it returns
> a non-negative integer if it succeeds, so that should
> be safe.
>
> It seems that the send() family of system calls can
> return a negative value that is a success, but only
> if passed a sufficiently large buffer size so that
> it will become negative when casted to a signed
> integer. These functions could need a little
> investigation to either show that it cannot happen
> in practice, or do something about it so that it
> cannot happen in practice (perhaps the error
> should be checked in the old way for those
> calls if those calls are not known for returning
> strange error codes).

This can't happen, because generally if a size_t value is passed
that's too large to fit into the ssize_t return value, it's an error
and errno is set to EINVAL.

> Steve, I suggest that you revise your commit
> message to summarize the arguments in your
> email and also a justification why the change
> is safe (similar to what I have done in this
> email, but expanding on it).
>
> Putting that information in the commit message
> has two advantages: It will be easier for us to
> review the patch since all relevant information
> can be found in one place and if anyone in the
> future stumbles upon this commit and wonders
> why it was made, (s)he will immediately find all
> information in one place and will not have to
> waste time searching through the mailing
> list archives.

Good point. I've amended my commit to include this information.

thanks,
--steve