Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Networking updates #3693

Draft
wants to merge 16 commits into
base: development
Choose a base branch
from
Draft

Conversation

d-torrance
Copy link
Member

I've been looking through our networking code while I work on the language server and have made a few changes:

  • We had quite a few things hidden behind #ifdef HAVE_function clauses for networking functions that have been POSIX standard for over twenty years, so we remove those. We also remove a bunch of checks for functions that generated macros we weren't actually using.
  • We refactor the error handing for getaddrinfo, which previously used a global variable and I don't think it was thread safe and remove a few unused error handling functions.
  • We loop through all the addresses returned by getaddrinfo and use the first one that works rather than just hoping that the very first one works.
  • Add support for openOutAppend to sockets and pipes. It does the exact same thing as openOut, but our current behavior for something like openOutAppend "$localhost:12345" << "Hello, world!" << close is to actually write to a file named $localhost:12345 rather than send a message to port 12345.

This is a draft for now, as I'll probably do more with it (e.g., documentation).

It was added to the POSIX standard in 2001 and should be available on
all modern systems.

Since we no longer need to roll our own host_address, which used
inet_addr, we can drop the check for arpa/inet.h.
The "PF" is for "protocol family" (AF is "address family").  According
to the GNU libc manual:

"The corresponding namespace designator symbol PF_UNSPEC exists for
completeness, but there is no reason to use it in a program."
It was a thin wrapper around getaddrinfo that used a global variable
to store the error message when an error was raised.  This likely
wasn't thread safe.

Instead, we determine the error message a bit later, when it's time
to actually print it, without any global variables.  This requires a
little bit of logic to distinguish between getaddrinfo errors and
non-getaddrinfo errors.  In particular, we shift the error codes of
the getaddrinfo errors and then shift back later when we need them.
In particular:

* Use memset to initialize the hints struct instead of static (for
  thread safety)
* Actually use a hints struct in opensocket
* Loop through all the elements of the linked list returned by
  getaddrinfo until we find one that works rather than just crossing
  our fingers and going with the first one
* Use the info in the struct returned by getaddrinfo in our call to
  socket rather than hardcoding everything
* With new for loop logic, we only need a single call to freeaddrinfo.

Code heavily inspired by Beej's Guide to Network Programming:
https://beej.us/guide/bgnet
The value of status will only be accurate if the process has changed
state, i.e., if waitpid returned a positive value.  So in particular,
we won't be able to trust that fix_status will return -2 in this case
as given in the documentation.  So we return -2 manually in this case.
So many ternary conditional operators...

Also put #ifdef's around the non-POSIX WCOREDUMP as recommended in the
waitpid manpage.
Also remove some "is Error" cases -- user can't pass an Error!
For the "method already installed" error message to look okay (i.e.,
not display "MutableHashTable{...}"), we turn nullaryMethods into a
Type, which kind of makes sense anyway since it's the object we
install these methods in.
Previously, something like openOutAppend "$localhost:12345" or
openOutAppend "!cat" would cause us to write to files with these names
rather than the expected behavior.

The "Append" really doesn't mean anything -- behavior is identical to
openOut in these cases.
@d-torrance d-torrance force-pushed the getaddrinfo branch 2 times, most recently from 2365e64 to c18ea36 Compare March 16, 2025 04:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant