[PATCH v2] ws2_32: Drop dependencies on system getprotoby(name|number) functions
Zebediah Figura
z.figura12 at gmail.com
Tue Jul 14 10:40:37 CDT 2020
On 7/14/20 12:35 AM, Alex Henrie wrote:
> Signed-off-by: Alex Henrie <alexhenrie24 at gmail.com>
> ---
> v2:
> - Keep original protocol table format
> - Implement getprotobynumber using binary search
> ---
> configure.ac | 2 -
> dlls/ws2_32/socket.c | 76 ++++++++-------------------
> dlls/ws2_32/tests/protocol.c | 99 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 120 insertions(+), 57 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 4829648c3a..9002ce8d60 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -2245,8 +2245,6 @@ AC_CHECK_FUNCS(\
> getaddrinfo \
> getnameinfo \
> getnetbyname \
> - getprotobyname \
> - getprotobynumber \
> getservbyport \
> )
>
> diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c
> index 69fbb37af2..a860258beb 100644
> --- a/dlls/ws2_32/socket.c
> +++ b/dlls/ws2_32/socket.c
> @@ -6507,58 +6507,26 @@ struct WS_hostent* WINAPI WS_gethostbyname(const char* name)
>
> static const struct { int prot; const char *names[3]; } protocols[] =
> {
> + /* keep this list sorted by number for binary search */
This seems to me like it should be a separate patch.
> { 0, { "ip", "IP" }},
> { 1, { "icmp", "ICMP" }},
> - { 2, { "igmp", "IGMP" }},
> { 3, { "ggp", "GGP" }},
> { 6, { "tcp", "TCP" }},
> { 8, { "egp", "EGP" }},
> - { 9, { "igp", "IGP" }},
> { 12, { "pup", "PUP" }},
> { 17, { "udp", "UDP" }},
> { 20, { "hmp", "HMP" }},
> { 22, { "xns-idp", "XNS-IDP" }},
> { 27, { "rdp", "RDP" }},
> - { 29, { "iso-tp4", "ISO-TP4" }},
> - { 33, { "dccp", "DCCP" }},
> - { 36, { "xtp", "XTP" }},
> - { 37, { "ddp", "DDP" }},
> - { 38, { "idpr-cmtp", "IDPR-CMTP" }},
> { 41, { "ipv6", "IPv6" }},
> { 43, { "ipv6-route", "IPv6-Route" }},
> { 44, { "ipv6-frag", "IPv6-Frag" }},
> - { 45, { "idrp", "IDRP" }},
> - { 46, { "rsvp", "RSVP" }},
> - { 47, { "gre", "GRE" }},
> { 50, { "esp", "ESP" }},
> { 51, { "ah", "AH" }},
> - { 57, { "skip", "SKIP" }},
> { 58, { "ipv6-icmp", "IPv6-ICMP" }},
> { 59, { "ipv6-nonxt", "IPv6-NoNxt" }},
> { 60, { "ipv6-opts", "IPv6-Opts" }},
> { 66, { "rvd", "RVD" }},
> - { 73, { "rspf", "RSPF" }},
> - { 81, { "vmtp", "VMTP" }},
> - { 88, { "eigrp", "EIGRP" }},
> - { 89, { "ospf", "OSPFIGP" }},
> - { 93, { "ax.25", "AX.25" }},
> - { 94, { "ipip", "IPIP" }},
> - { 97, { "etherip", "ETHERIP" }},
> - { 98, { "encap", "ENCAP" }},
> - { 103, { "pim", "PIM" }},
> - { 108, { "ipcomp", "IPCOMP" }},
> - { 112, { "vrrp", "VRRP" }},
> - { 115, { "l2tp", "L2TP" }},
> - { 124, { "isis", "ISIS" }},
> - { 132, { "sctp", "SCTP" }},
> - { 133, { "fc", "FC" }},
> - { 135, { "mobility-header", "Mobility-Header" }},
> - { 136, { "udplite", "UDPLite" }},
> - { 137, { "mpls-in-ip", "MPLS-in-IP" }},
> - { 139, { "hip", "HIP" }},
> - { 140, { "shim6", "Shim6" }},
> - { 141, { "wesp", "WESP" }},
> - { 142, { "rohc", "ROHC" }},
> };
>
> /***********************************************************************
> @@ -6567,24 +6535,18 @@ static const struct { int prot; const char *names[3]; } protocols[] =
> struct WS_protoent* WINAPI WS_getprotobyname(const char* name)
> {
> struct WS_protoent* retval = NULL;
> -#ifdef HAVE_GETPROTOBYNAME
> - struct protoent* proto;
> - EnterCriticalSection( &csWSgetXXXbyYYY );
> - if( (proto = getprotobyname(name)) != NULL )
> - retval = WS_create_pe( proto->p_name, proto->p_aliases, proto->p_proto );
> - LeaveCriticalSection( &csWSgetXXXbyYYY );
> -#endif
> - if (!retval)
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(protocols); i++)
> {
> - unsigned int i;
> - for (i = 0; i < ARRAY_SIZE(protocols); i++)
> + if (_strnicmp( protocols[i].names[0], name, -1 ) == 0)
> {
> - if (_strnicmp( protocols[i].names[0], name, -1 )) continue;
> retval = WS_create_pe( protocols[i].names[0], (char **)protocols[i].names + 1,
> protocols[i].prot );
> break;
> }
> }
> +
> if (!retval)
> {
> WARN( "protocol %s not found\n", debugstr_a(name) );
> @@ -6601,24 +6563,28 @@ struct WS_protoent* WINAPI WS_getprotobyname(const char* name)
> struct WS_protoent* WINAPI WS_getprotobynumber(int number)
> {
> struct WS_protoent* retval = NULL;
> -#ifdef HAVE_GETPROTOBYNUMBER
> - struct protoent* proto;
> - EnterCriticalSection( &csWSgetXXXbyYYY );
> - if( (proto = getprotobynumber(number)) != NULL )
> - retval = WS_create_pe( proto->p_name, proto->p_aliases, proto->p_proto );
> - LeaveCriticalSection( &csWSgetXXXbyYYY );
> -#endif
> - if (!retval)
> + int start = 0, end = ARRAY_SIZE(protocols) - 1, i;
> +
> + do
> {
> - unsigned int i;
> - for (i = 0; i < ARRAY_SIZE(protocols); i++)
> + i = (start + end) / 2;
> + if (number < protocols[i].prot)
> + {
> + end = i - 1;
> + }
> + else if (number > protocols[i].prot)
> + {
> + start = i + 1;
> + }
> + else
> {
> - if (protocols[i].prot != number) continue;
> retval = WS_create_pe( protocols[i].names[0], (char **)protocols[i].names + 1,
> protocols[i].prot );
> break;
> }
> }
> + while (start <= end);
> +
If binary search is worth implementing, can we just use bsearch() to do it?
> if (!retval)
> {
> WARN( "protocol %d not found\n", number );
> diff --git a/dlls/ws2_32/tests/protocol.c b/dlls/ws2_32/tests/protocol.c
> index 99bd1373a9..4cc7770dad 100644
> --- a/dlls/ws2_32/tests/protocol.c
> +++ b/dlls/ws2_32/tests/protocol.c
> @@ -192,6 +192,103 @@ static void test_WSAEnumProtocolsW(void)
> }
> }
>
> +struct protocol
> +{
> + int prot;
> + const char *names[3];
I guess this was copied from the implementation, but surely this can
just be names[2]?
> + BOOL missing_from_xp;
> +};
> +
> +static const struct protocol protocols[] =
> +{
> + /* keep this list sorted by number for intelligent linear search */
> + { 0, { "ip", "IP" }},
> + { 1, { "icmp", "ICMP" }},
> + { 3, { "ggp", "GGP" }},
> + { 6, { "tcp", "TCP" }},
> + { 8, { "egp", "EGP" }},
> + { 12, { "pup", "PUP" }},
> + { 17, { "udp", "UDP" }},
> + { 20, { "hmp", "HMP" }},
> + { 22, { "xns-idp", "XNS-IDP" }},
> + { 27, { "rdp", "RDP" }},
> + { 41, { "ipv6", "IPv6" }, TRUE},
> + { 43, { "ipv6-route", "IPv6-Route" }, TRUE},
> + { 44, { "ipv6-frag", "IPv6-Frag" }, TRUE},
> + { 50, { "esp", "ESP" }},
> + { 51, { "ah", "AH" }},
> + { 58, { "ipv6-icmp", "IPv6-ICMP" }, TRUE},
> + { 59, { "ipv6-nonxt", "IPv6-NoNxt" }, TRUE},
> + { 60, { "ipv6-opts", "IPv6-Opts" }, TRUE},
> + { 66, { "rvd", "RVD" }},
> + { }
If you're using ARRAY_SIZE, does this need to be null-terminated?
> +};
> +
> +void test_getprotobyname(void)
> +{
> + struct protoent *ent;
> + char all_caps_name[16];
> + int i, j;
> +
> + for (i = 0; i < ARRAY_SIZE(protocols) - 1; i++)
> + {
> + for (j = 0; protocols[i].names[j]; j++)
> + {
> + ent = getprotobyname(protocols[i].names[j]);
> + ok((ent && ent->p_proto == protocols[i].prot) || broken(!ent && protocols[i].missing_from_xp),
> + "Expected %s to be protocol number %d, got %d\n",
> + wine_dbgstr_a(protocols[i].names[j]), protocols[i].prot, ent ? ent->p_proto : -1);
> + }
> +
> + if (protocols[i].names[0])
Isn't this condition always true?
> + {
> + for (j = 0; protocols[i].names[0][j]; j++)
> + all_caps_name[j] = toupper(protocols[i].names[0][j]);
> + all_caps_name[j] = 0;
> + ent = getprotobyname(all_caps_name);
> + ok((ent && ent->p_proto == protocols[i].prot) || broken(!ent && protocols[i].missing_from_xp),
> + "Expected %s to be protocol number %d, got %d\n",
> + wine_dbgstr_a(all_caps_name), protocols[i].prot, ent ? ent->p_proto : -1);
> + }
> + }
> +}
> +
> +void test_getprotobynumber(void)
> +{
> + struct protoent *ent;
> + const struct protocol *ref = &protocols[0];
> + int i, j;
> +
> + for (i = -1; i <= 256; i++)
> + {
> + ent = getprotobynumber(i);
> +
> + if (i != ref->prot)
> + {
> + ok(!ent,
This seems like an odd place for a line break.
> + "Expected protocol number %d to be undefined, got %s\n",
> + i, wine_dbgstr_a(ent ? ent->p_name : NULL));
> + continue;
> + }
> +
> + ok((ent && ent->p_name && strcmp(ent->p_name, ref->names[0]) == 0) ||
> + broken(!ent && ref->missing_from_xp),
> + "Expected protocol number %d to be %s, got %s\n",
> + i, ref->names[0], wine_dbgstr_a(ent ? ent->p_name : NULL));
> +
> + for (j = 0; ref->names[1+j]; j++)
> + {
> + ok((ent && ent->p_aliases && ent->p_aliases[j] &&
> + strcmp(ent->p_aliases[j], ref->names[1+j]) == 0) ||
> + broken(!ent && ref->missing_from_xp),
> + "Expected protocol number %d alias %d to be %s, got %s\n",
> + i, j, ref->names[0], wine_dbgstr_a(ent && ent->p_aliases ? ent->p_aliases[j] : NULL));
> + }
> +
> + ref++;
> + }
I guess this test structure isn't too confusing on examination, but it
might be clearer just to have a semi-reimplementation of
getprotobynumber() in the tests, to which you compare the actual data.
> +}
> +
> START_TEST( protocol )
> {
> WSADATA data;
> @@ -201,4 +298,6 @@ START_TEST( protocol )
>
> test_WSAEnumProtocolsA();
> test_WSAEnumProtocolsW();
> + test_getprotobyname();
> + test_getprotobynumber();
> }
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20200714/82e15fe9/attachment.sig>
More information about the wine-devel
mailing list