Ticket #548 (closed RFC: invalid)

Opened 6 years ago

Last modified 4 years ago

Error handling in socket system

Reported by: whiteknight Owned by:
Priority: normal Milestone:
Component: core Version:
Severity: medium Keywords: socket
Cc: Language:
Patch status: Platform: all

Description

Browsing through the socket system, I see a few places where error conditions don't seem to be handled in a way consistent with the rest of Parrot. Some examples:

src/io/socket_win32.c:get_sockaddr_in(). there are a few "XXX" comments here about poor error handling, and a bare fprintf(stderr, ...) call that should probably be replaced with a Parrot_warn() or a thrown exception.

Same thing in src/io/socket_unit.c:get_sockaddr_in().

Various platform-specific functions are returning platform-specific error codes, as is evidenced in places like src/io/socket_api.c:Parrot_io_socket_is_closed(). We should have a more unified way to say whether a function has succeeded or failed without using a large ugly #ifdef/#endif chain

Change History

Changed 6 years ago by Infinoid

Some of the issues you've raised are not specific to sockets, but affect the whole I/O subsystem.

That #ifdef/#endif chain isn't checking whether a function succeeded per se, it's checking whether the os_handle is still at its default value. See a similar ifdef/endif chain in Filehandle.init(), where those default values were assigned.

It seems to me there are some elements of the sockets API which don't need to be socket-specific at all. In this case, Parrot_io_socket_is_closed could just call PIO_IS_CLOSED. But if you look at how the latter is implemented, it calls one of [Parrot_io_is_closed_unix, Parrot_io_is_closed_win32, Parrot_io_is_closed_portable], each of which has one if() statement. This is equally as bad as the ifdef/endif chain in my opinion, and even more spread out. Do you have any suggestions on what the "right" way to do this is?

How about a #define indicating what an invalid handle is for the current platform (called "PARROT_INVALID_HANDLE_VALUE" perhaps), and a simple equality test against that?

Changed 4 years ago by whiteknight

  • status changed from new to closed
  • resolution set to invalid

This RFC hasn't attracted any attention in 20 months, and I think a different ticket would be better at addressing some of the issues touched upon here. Closing ticket.

Note: See TracTickets for help on using tickets.