[erlang-patches] Montavista system call error handling patch
Mikael Pettersson
mikpe@REDACTED
Fri May 14 00:12:11 CEST 2010
Steve Vinoski writes:
> We've recently experienced a 100% repeatable case of R13B02-1 and
> R13B04 beam core dumps on a Montavista Linux system running on a
> Cavium Octeon processor. This is technically not a problem with the
> Erlang VM but rather is a case where certain socket-related calls in
> this version of Montavista Linux, based on a 2.6.21 Linux kernel, are
> indicating failure by returning negative numbers with large absolute
> values, such as negative 0x40000, rather than returning -1 to indicate
> failure as they should. The Erlang TCP code expects system calls to
> return -1 to indicate errors and compares directly against that value,
> so it ends up treating these negative return values as successful and
> adds them to internal pointers as bytes written, offsets, etc. This in
> turn corrupts these pointers and causes beam to dump core.
>
> This patch introduces a portability macro to test for system call
> failure within inet_drv.c:
>
> git fetch git://github.com/vinoski/otp.git socket_error_portability
I'm not happy with this patch.
First, you're changing (weakening) error checking on all non-Win32
platforms, not just the apparently horribly broken MVL. At a minimum
the new IS_SOCKET_ERROR() macro should be written as
#if <whatever the test for Montavista's butchered Linux is>
#define IS_SOCKET_ERROR(val) ((val) < 0)
#else
#define IS_SOCKET_ERROR(val) ((val) == SOCKET_ERROR)
#endif
Second, the change needs a big comment. Otherwise someone who's ever
read the POSIX or SuS specs will want to clean it up and turn it back
to the original, not realising the reason for the out-of-spec error check.
(Personally I'd rather wrap the broken socket calls with functions that
convert them to sane APIs, in this case just fix up error returns, than
to force all users to adapt to the breakage. But lots of code in erts
could do with that sort of cleanup, so I'm not going to complain if you
don't want to do it that way.)
/Mikael
More information about the erlang-patches
mailing list