Fix ./configure EUI check

[timp87]

Fix missing header sys/socket.h:

configure:29217: checking for struct arpreq.arp_pa
.../net/if_arp.h: error: field has incomplete type 'struct sockaddr'

Fix ac_aggr.arp_pa test, replacing it with a test of a data member
that Squid code is actually using:

conftest.cpp:112:5: error: value of type 'struct sockaddr' is not
    contextually convertible to 'bool'

Discovered while porting Squid v7 to FreeBSD.

[timp87]

1. The EUI check uses #include <sys/socket.h> header placed under #if HAVE_SYS_SOCKET_H condition https://github.com/squid-cache/squid/blob/SQUID_7_3/configure.ac#L1003-L1005
   Full error

configure:29217: checking for struct arpreq.arp_pa
configure:29217: c++ -std=c++17 -c  -Wall -Wextra -Wno-unused-private-field -Wno-implicit-fallthrough -Wpointer-arith -Wwrite-strings -Wcomments -Wshadow -Wmissing-declarations -Woverloaded-virtual  -D_REENTRANT -DBOOL_DEFINED -isystem /u
sr/local/include conftest.cpp >&5
In file included from conftest.cpp:103:
/usr/include/net/if_arp.h:89:18: error: field has incomplete type 'struct sockaddr'
   89 |         struct  sockaddr arp_pa;                /* protocol address */
      |                          ^
/usr/include/net/if_arp.h:89:9: note: forward declaration of 'sockaddr'
   89 |         struct  sockaddr arp_pa;                /* protocol address */
      |                 ^
/usr/include/net/if_arp.h:90:18: error: field has incomplete type 'struct sockaddr'
   90 |         struct  sockaddr arp_ha;                /* hardware address */
      |                          ^
/usr/include/net/if_arp.h:89:9: note: forward declaration of 'sockaddr'
   89 |         struct  sockaddr arp_pa;                /* protocol address */
      |                 ^
2 errors generated.

The problem here is that HAVE_SYS_SOCKET_H is not defined yet at that point, so it's skipped and the check fails.
I'm not sure how to fix it better like remove #if HAVE_SYS_SOCKET_H condition and let the header be included unconditionally, or include this header into some AC_CHECK_HEADERS a bit above, or something else. So please guide me how you wanted me this to do. For now I just put separate simple AC_CHECK_HEADERS
BTW, there is big AC_CHECK_HEADERS https://github.com/squid-cache/squid/blob/SQUID_7_3/configure.ac#L1758C1-L1758C17 where HAVE_SYS_SOCKET_H will be set, but it's below the test

2. Once I fixed previous error I get another:

configure:29224: checking for struct arpreq.arp_pa
configure:29224: c++ -std=c++17 -c  -Wall -Wextra -Wno-unused-private-field -Wno-implicit-fallthrough -Wpointer-arith -Wwrite-strings -Wcomments -Wshadow -Wmissing-declarations -Woverloaded-virtual  -D_REENTRANT -DBOOL_DEFINED -isystem /usr/local/include conftest.cpp >&5
conftest.cpp:112:5: error: value of type 'struct sockaddr' is not contextually convertible to 'bool'
  112 | if (ac_aggr.arp_pa)
      |     ^~~~~~~~~~~~~~
1 error generated.

So use particular member (sa_family) for the check.

[rousskov]

Thank you for the fix and useful triage details.

[rousskov]

The problem here is that HAVE_SYS_SOCKET_H is not defined yet at that point, so it's skipped and the check fails.

I'm not sure how to fix it better like remove #if HAVE_SYS_SOCKET_H condition and let the header be included unconditionally, or include this header into some AC_CHECK_HEADERS a bit above, or something else. So please guide me how you wanted me this to do

I think what you did is the right solution for this PR -- it matches how similar code solves similar problems. There may be a better, more comprehensive solution, but I would place that outside this small/focused PR scope.

This comment answers a question without requesting any changes.

[yadij]

IMO the checks here should be including our compat/types.h and compat/socket.h instead of the system headers. But that will still need this change first (along with other headers tested). So I am putting that on my TODO list for later, no need to do it here.

[rousskov]

IMO the checks here should be including our compat/types.h and compat/socket.h

I am against using compat/ sources in ./configure checks because that would imply that compat/ sources cannot reliably use ./configure results. However, there is now a better place for that discussion. Let's continue this conversation there.

[rousskov]

IMO the checks here should be including our compat/types.h and compat/socket.h

I am against using compat/ sources in ./configure checks because that would imply that compat/ sources cannot reliably use ./configure results. However, there is now a better place for that discussion. Let's continue this conversation there.

[rousskov]

I adjusted the title to make it specific to ./configure again (and to avoid introducing EUI prefix).

[kinkie]

let's get this fixed for now, and we can improve later

[squidadm]

queued for backport to v7