OpenSmalltalk / opensmalltalk-vm

Cross-platform virtual machine for Squeak, Pharo, Cuis, and Newspeak.
http://opensmalltalk.org/
Other
564 stars 111 forks source link

Remove socket file check in sqResolverGetAddressInfoHostSizeServiceSizeFlagsFamilyTypeProtocol #691

Closed nafiz1001 closed 4 months ago

nafiz1001 commented 4 months ago

I was able to build the project on Linux, and I verified that I could generate a (valid) socket address for a socket path with primitiveResolverGetAddressInfoHost:service:flags:family:type:protocol: regardless of the file's existence and even bind a socket to it with primitiveSocket:bindTo: (if the file does not exist).

This change is necessary to make primitiveResolverGetAddressInfoHost:service:flags:family:type:protocol: usable for generating socket addresses as a unix socket server written in Smalltalk.

dtlewis290 commented 4 months ago

Although I am not able to test this patch, I would recommend merging it. It will not harm any existing Socket functionality, and if Nafiz recommends it then we should trust his judgement.

For background, the AF_UNIX code was added by Ian Piumarta as part of his IPv6 work, which was a large update. The original commit for SocketPlugin was at http://squeakvm.org/cgi-bin/viewvc.cgi/squeak?view=revision&revision=1742 and the changes in that update can be seen at http://squeakvm.org/cgi-bin/viewvc.cgi/squeak/trunk/platforms/unix/src/vm/intplugins/SocketPlugin/SocketPlugin.c?r1=1742&r2=1741&pathrev=1742

The VM level support for IPv6 was not added to the Squeak image until much later, and is probably incomplete even today. the AF_UNIX socket support may be an example of this. If it requires a bit of tweaking on the VM side, I would not be surprised.

I should note that I do not know the intent of the original check for "if (!stat(servName, &st) && (st.st_mode & S_IFSOCK))" so if anyone has better insight please speak up.

From my POV, +1 for merging this PR.