In Socket.hpp, prevent warnings about sock_type comparison

[league]

It looks like we're trying to use enum sock_type to impose a more limited type on the int specified by ::socket. But it wasn't all the way supported, because the constructor took a general int (doesn't seem to be used that way within bifrost) and there were several comparisons between SOCK_STREAM (respectively SOCK_DGRAM) integer constants and enum-enabled BF_SOCK_STREAM (BF_SOCK_DGRAM).

So this takes that goal further, using the enum version only. An alternative though (in case some other client code is using a general integer for other socket types) is just to eliminate enum sock_type and (if necessary) throw an exception if passed a SOCK_* constant other than those we support.

The warnings were...

In file included from address.cpp:30:
Socket.hpp: In member function 'void
   Socket::bind(sockaddr_storage, int)':
Socket.hpp:584:12: warning: comparison between
   'enum Socket::sock_type' and 'enum
   __socket_type' [8;;
   https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wenum-compare
   -Wenum-compare8;;]
  584 |  if( _type == SOCK_STREAM ) {
      |            ^
Socket.hpp:584:12: warning: comparison between
   types 'Socket::sock_type' and '__socket_type' [
   8;;
   https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wsign-compare
   -Wsign-compare8;;]
Socket.hpp: In member function 'void
   Socket::connect(sockaddr_storage)':
Socket.hpp:596:26: warning: comparison between
   'enum Socket::sock_type' and 'enum
   __socket_type' [8;;
   https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wenum-compare
   -Wenum-compare8;;]
  596 |                    _type == SOCK_DGRAM &&
      |                          ^
Socket.hpp:596:26: warning: comparison between
   types 'Socket::sock_type' and '__socket_type' [
   8;;
   https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wsign-compare
   -Wsign-compare8;;]

[codecov-commenter]

Codecov Report

Merging #170 (b6f8822) into master (4dd48d6) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #170   +/-   ##
=======================================
  Coverage   58.36%   58.36%           
=======================================
  Files          67       67           
  Lines        5753     5753           
=======================================
  Hits         3358     3358           
  Misses       2395     2395           

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4dd48d6...b6f8822. Read the comment docs.

[jaycedowell]

I remember adding in the BF_SOCK_... enum as part of the MacOS support work for autoconf but I don't remember what caused me to do it. If this isn't really needed I would be in favor of converting ::socket back to int for consistency and throwing an exception for unsupported values.

[league]

Yeah, here's a simplification by removing enum sock_type. I think this should be fine. It shouldn't throw any warnings, and removes the redundant constants for BF_SOCK_*. If one needs the SOCK_* constants from Python code, they are in socket module.

[league]

This all looks like an improvement now… pleased to be rid of those ifdefs too. Fine to ship, IMO.

Re whitespace, I'd def be happy with spaces-only. Some projects use an "editor config" to help promote consistency. Less obtrusive than a commit hook and less opinionated than a full-on reformatter. I never bothered to get emacs to read and adapt to these, but I would if we use it. Also, a tree-wide untabify (or any other reformat) would best be done when there are very few unmerged branches lying around.

[jaycedowell]

We should definitely untabify but I'm happy to kick that can down the road.