Resubmitting the patches with the requested commit message changes
Also, change a few other debug output spots for consitency --- winsup/cygwin/libc/minires.c | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/winsup/cygwin/libc/minires.c b/winsup/cygwin/libc/minires.c index 0979daae3..0cf9efd9b 100644 --- a/winsup/cygwin/libc/minires.c +++ b/winsup/cygwin/libc/minires.c @@ -73,7 +73,7 @@ void minires_get_search(char * string, res_state statp) statp->dnsrch[j] = strcpy(ptr, words[j]); statp->dnsrch[j+1] = NULL; ptr += sizes[j]; - DPRINTF(debug, "search \"%s\"\n", words[j]); + DPRINTF(debug, "registry search \"%s\"\n", words[j]); } else if (j < MAXDNSRCH + 1) DPRINTF(debug, "no space for \"%s\"\n", words[j]); @@ -170,7 +170,7 @@ static void get_resolv(res_state statp) have_address = (statp->nscount != 0); while ( fgets(line, sizeof(line), fd) != 0) { - DPRINTF(debug, "resolv.conf %s", line); + DPRINTF(debug, _PATH_RESCONF " line: %s", line); if ((i = scanline(line, words, sizes, DIM(words))) > 0) { if (!have_address && !strncasecmp("nameserver", words[0], sizes[0])) { @@ -186,7 +186,7 @@ static void get_resolv(res_state statp) else { statp->nsaddr_list[ns++].sin_addr.s_addr = address; statp->nscount++; - DPRINTF(debug, "server \"%s\"\n", words[j]); + DPRINTF(debug, "nameserver \"%s\"\n", words[j]); } } } @@ -254,6 +254,8 @@ static int open_sock(struct sockaddr_in *CliAddr, int debug) DPRINTF(debug, "bind: %s\n", strerror(errno)); return -1; } + + DPRINTF(debug, "UDP socket sockfd %d\n", fd); return fd; } \f @@ -352,10 +354,10 @@ static int get_tcp_buf(int fd, unsigned char *buf, int size, int debug) int res; while (size > 0) { if ((res = read(fd, buf, size)) < 0) { - DPRINTF(debug, "read: %s\n", strerror(errno)); + DPRINTF(debug, "read(TCP): %s\n", strerror(errno)); return -1; } - DPRINTF(debug, "read %d out of %d\n", res, size); + DPRINTF(debug, "read(TCP) %d out of %d\n", res, size); size -= res; buf += res; } @@ -384,7 +386,7 @@ static int get_tcp(struct sockaddr_in *CliAddr, } if (cygwin_connect(fd, (struct sockaddr *) CliAddr, sizeof(* CliAddr)) < 0) { - DPRINTF(debug, "connect: %s\n", strerror(errno)); + DPRINTF(debug, "connect(TCP): %s\n", strerror(errno)); goto done; } @@ -392,7 +394,7 @@ static int get_tcp(struct sockaddr_in *CliAddr, len_buf.len = htons(MsgLength); if (write(fd, len_buf.buf, sizeof(len_buf)) != sizeof(len_buf) || write(fd, MsgPtr, MsgLength) != MsgLength) { - DPRINTF(debug, "write: %s\n", strerror(errno)); + DPRINTF(debug, "write(TCP): %s\n", strerror(errno)); goto done; } @@ -503,8 +505,9 @@ int res_nsend( res_state statp, const unsigned char * MsgPtr, rslt = cygwin_sendto(statp->sockfd, MsgPtr, MsgLength, 0, (struct sockaddr *) &statp->nsaddr_list[wServ], sizeof(struct sockaddr_in)); - DPRINTF(debug, "sendto: server %08x sockfd %d %s\n", - statp->nsaddr_list[wServ].sin_addr.s_addr, + DPRINTF(debug, "sendto: server %08X:%hu sockfd %d %s\n", + ntohl(statp->nsaddr_list[wServ].sin_addr.s_addr), + ntohs(statp->nsaddr_list[wServ].sin_port), statp->sockfd, (rslt == MsgLength)?"OK":strerror(errno)); if (rslt != MsgLength) { statp->res_h_errno = NETDB_INTERNAL; @@ -519,12 +522,14 @@ int res_nsend( res_state statp, const unsigned char * MsgPtr, timeOut.tv_usec = 0; rslt = cygwin_select(statp->sockfd + 1, &fdset_read, NULL, NULL, &timeOut); if ( rslt == 0 ) { /* Timeout */ - DPRINTF(statp->options & RES_DEBUG, "timeout for server %08x\n", - statp->nsaddr_list[wServ].sin_addr.s_addr); + DPRINTF(statp->options & RES_DEBUG, "timeout for server %08X:%hu sockfd %d\n", + ntohl(statp->nsaddr_list[wServ].sin_addr.s_addr), + ntohs(statp->nsaddr_list[wServ].sin_port), + statp->sockfd); continue; } else if ((rslt != 1) || (FD_ISSET(statp->sockfd, &fdset_read) == 0)) { - DPRINTF(debug, "select: %s\n", strerror(errno)); + DPRINTF(debug, "select sockfd %d: %s\n", statp->sockfd, strerror(errno)); statp->res_h_errno = NETDB_INTERNAL; return -1; } @@ -533,11 +538,14 @@ int res_nsend( res_state statp, const unsigned char * MsgPtr, rslt = cygwin_recvfrom(statp->sockfd, AnsPtr, AnsLength, 0, (struct sockaddr *) & dnsSockAddr, & addrLen); if (rslt <= 0) { - DPRINTF(debug, "recvfrom: %s\n", strerror(errno)); + DPRINTF(debug, "recvfrom sockfd %d: %s\n", statp->sockfd, strerror(errno)); statp->res_h_errno = NETDB_INTERNAL; return -1; } - DPRINTF(debug, "recvfrom: %d bytes from %08x\n", rslt, dnsSockAddr.sin_addr.s_addr); + DPRINTF(debug, "recvfrom: %d bytes from %08X:%hu sockfd %d\n", rslt, + ntohl(dnsSockAddr.sin_addr.s_addr), + ntohs(dnsSockAddr.sin_port), + statp->sockfd); /* Prepare to retry with tcp */ -- 2.33.0
--- winsup/cygwin/libc/minires.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/winsup/cygwin/libc/minires.c b/winsup/cygwin/libc/minires.c index 0cf9efd9b..fdc6087f5 100644 --- a/winsup/cygwin/libc/minires.c +++ b/winsup/cygwin/libc/minires.c @@ -86,12 +86,12 @@ Read options ***********************************************************************/ -static void get_options(res_state statp, int i, char **words) +static void get_options(res_state statp, int n, char **words) { char *ptr; - int value; + int i, value; - while (i-- > 0) { + for (i = 0; i < n; ++i) { if (!strcasecmp("debug", words[i])) { statp->options |= RES_DEBUG; DPRINTF(statp->options & RES_DEBUG, "%s: 1\n", words[i]); @@ -208,8 +208,10 @@ static void get_resolv(res_state statp) } } /* Options line */ - else if (!strncasecmp("options", words[0], sizes[0])) + else if (!strncasecmp("options", words[0], sizes[0])) { get_options(statp, i - 1, &words[1]); + debug = statp->options & RES_DEBUG; + } } } fclose(fd); -- 2.33.0
--- winsup/cygwin/libc/minires-os-if.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/winsup/cygwin/libc/minires-os-if.c b/winsup/cygwin/libc/minires-os-if.c index 6e17de0b8..f71178b96 100644 --- a/winsup/cygwin/libc/minires-os-if.c +++ b/winsup/cygwin/libc/minires-os-if.c @@ -210,7 +210,7 @@ static int cygwin_query(res_state statp, const char * DomName, int Class, int Ty #define NO_DATA 4 /* Valid name, no data record of requested type */ #endif - DPRINTF(debug, "DnsQuery: %lu (Windows)\n", res); + DPRINTF(debug, "DnsQuery: %u (Windows)\n", res); if (res) { switch (res) { case ERROR_INVALID_NAME: @@ -236,7 +236,7 @@ static int cygwin_query(res_state statp, const char * DomName, int Class, int Ty statp->res_h_errno = NO_DATA; break; default: - DPRINTF(debug, "Unknown code %lu for %s %d\n", res, DomName, Type); + DPRINTF(debug, "Unknown code %u for %s %d\n", res, DomName, Type); statp->res_h_errno = NO_RECOVERY; break; } @@ -442,7 +442,7 @@ void get_dns_info(res_state statp) /* First call to get the buffer length we need */ dwRetVal = GetNetworkParams((FIXED_INFO *) 0, &ulOutBufLen); if (dwRetVal != ERROR_BUFFER_OVERFLOW) { - DPRINTF(debug, "GetNetworkParams: error %lu (Windows)\n", dwRetVal); + DPRINTF(debug, "GetNetworkParams: error %u (Windows)\n", dwRetVal); goto use_registry; } if ((pFixedInfo = (FIXED_INFO *) alloca(ulOutBufLen)) == 0) { @@ -450,7 +450,7 @@ void get_dns_info(res_state statp) goto use_registry; } if ((dwRetVal = GetNetworkParams(pFixedInfo, & ulOutBufLen))) { - DPRINTF(debug, "GetNetworkParams: error %lu (Windows)\n", dwRetVal); + DPRINTF(debug, "GetNetworkParams: error %u (Windows)\n", dwRetVal); goto use_registry; } -- 2.33.0
--- winsup/cygwin/libc/minires-os-if.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/winsup/cygwin/libc/minires-os-if.c b/winsup/cygwin/libc/minires-os-if.c index f71178b96..6b4c5e33e 100644 --- a/winsup/cygwin/libc/minires-os-if.c +++ b/winsup/cygwin/libc/minires-os-if.c @@ -165,7 +165,7 @@ static unsigned char * write_record(unsigned char * ptr, PDNS_RECORD rr, default: { unsigned int len = rr->wDataLength; - DPRINTF(debug, "No structure for wType %d\n", rr->wType); + DPRINTF(debug, "No structure for wType %u\n", rr->wType); if (ptr + len <= EndPtr) memcpy(ptr, (char *) &rr->Data, len); ptr += len; @@ -210,7 +210,7 @@ static int cygwin_query(res_state statp, const char * DomName, int Class, int Ty #define NO_DATA 4 /* Valid name, no data record of requested type */ #endif - DPRINTF(debug, "DnsQuery: %u (Windows)\n", res); + DPRINTF(debug, "DnsQuery for \"%s\" %d: %u (Windows)\n", DomName, Type, res); if (res) { switch (res) { case ERROR_INVALID_NAME: @@ -236,7 +236,7 @@ static int cygwin_query(res_state statp, const char * DomName, int Class, int Ty statp->res_h_errno = NO_DATA; break; default: - DPRINTF(debug, "Unknown code %u for %s %d\n", res, DomName, Type); + DPRINTF(debug, "Unknown code %u\n", res); statp->res_h_errno = NO_RECOVERY; break; } @@ -274,7 +274,7 @@ static int cygwin_query(res_state statp, const char * DomName, int Class, int Ty /* Check the records are in correct section order */ if ((rr->Flags.DW & 0x3) < section) { - DPRINTF(debug, "Unexpected section order %s %d\n", DomName, Type); + DPRINTF(debug, "Unexpected section order for \"%s\" %d\n", DomName, Type); continue; } section = rr->Flags.DW & 0x3; @@ -324,7 +324,7 @@ static void get_registry_dns_items(PUNICODE_STRING in, res_state statp, srch++); *srch++ = 0; if (numAddresses < DIM(statp->nsaddr_list)) { - DPRINTF(debug, "server \"%s\"\n", ap); + DPRINTF(debug, "registry server \"%s\"\n", ap); statp->nsaddr_list[numAddresses].sin_addr.s_addr = cygwin_inet_addr((char *) ap); if ( statp->nsaddr_list[numAddresses].sin_addr.s_addr != 0 ) numAddresses++; @@ -355,7 +355,7 @@ static void get_registry_dns(res_state statp) NTSTATUS status; const PCWSTR keyName = L"Tcpip\\Parameters"; - DPRINTF(statp->options & RES_DEBUG, "key %ls\n", keyName); + DPRINTF(statp->options & RES_DEBUG, "key \"%ls\"\n", keyName); status = RtlCheckRegistryKey (RTL_REGISTRY_SERVICES, keyName); if (!NT_SUCCESS (status)) { @@ -460,7 +460,7 @@ void get_dns_info(res_state statp) pIPAddr; pIPAddr = pIPAddr->Next) { if (numAddresses < DIM(statp->nsaddr_list)) { - DPRINTF(debug, "server \"%s\"\n", pIPAddr->IpAddress.String); + DPRINTF(debug, "params server \"%s\"\n", pIPAddr->IpAddress.String); statp->nsaddr_list[numAddresses].sin_addr.s_addr = cygwin_inet_addr(pIPAddr->IpAddress.String); if (statp->nsaddr_list[numAddresses].sin_addr.s_addr != 0) { numAddresses++; -- 2.33.0
AAAA records returned from Windows resolver were flagged as "No structure" in debug output because of being processed (although correctly) in the default catch-all case. This patch makes the AAAA records properly recognized. --- winsup/cygwin/libc/minires-os-if.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/winsup/cygwin/libc/minires-os-if.c b/winsup/cygwin/libc/minires-os-if.c index 6b4c5e33e..bb6786f6c 100644 --- a/winsup/cygwin/libc/minires-os-if.c +++ b/winsup/cygwin/libc/minires-os-if.c @@ -69,15 +69,14 @@ static unsigned char * write_record(unsigned char * ptr, PDNS_RECORD rr, switch(rr->wType) { case DNS_TYPE_A: + case DNS_TYPE_AAAA: { - u_int8_t * aptr = (u_int8_t *) & rr->Data.A.IpAddress; - if (ptr + 4 <= EndPtr) { - ptr[0] = aptr[0]; - ptr[1] = aptr[1]; - ptr[2] = aptr[2]; - ptr[3] = aptr[3]; - } - ptr += 4; + u_int8_t * aptr = rr->wType == DNS_TYPE_A + ? (u_int8_t *) & rr->Data.A.IpAddress : (u_int8_t *) & rr->Data.AAAA.Ip6Address; + int sz = rr->wType == DNS_TYPE_A ? NS_INADDRSZ/*4*/ : NS_IN6ADDRSZ/*16*/; + if (ptr + sz <= EndPtr) + memcpy(ptr, aptr, sz); + ptr += sz; break; } case DNS_TYPE_NS: -- 2.33.0
Hi Anton,
I pushed patches 1 and 3 to 5. I fixed the consitency typo
throughout.
As for this path 2:
On Jan 17 13:03, Anton Lavrentiev via Cygwin-patches wrote:
> ---
> winsup/cygwin/libc/minires.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/winsup/cygwin/libc/minires.c b/winsup/cygwin/libc/minires.c
> index 0cf9efd9b..fdc6087f5 100644
> --- a/winsup/cygwin/libc/minires.c
> +++ b/winsup/cygwin/libc/minires.c
> @@ -86,12 +86,12 @@ Read options
>
>
> ***********************************************************************/
> -static void get_options(res_state statp, int i, char **words)
> +static void get_options(res_state statp, int n, char **words)
> {
> char *ptr;
> - int value;
> + int i, value;
>
> - while (i-- > 0) {
> + for (i = 0; i < n; ++i) {
> if (!strcasecmp("debug", words[i])) {
> statp->options |= RES_DEBUG;
> DPRINTF(statp->options & RES_DEBUG, "%s: 1\n", words[i]);
> @@ -208,8 +208,10 @@ static void get_resolv(res_state statp)
> }
> }
> /* Options line */
> - else if (!strncasecmp("options", words[0], sizes[0]))
> + else if (!strncasecmp("options", words[0], sizes[0])) {
> get_options(statp, i - 1, &words[1]);
> + debug = statp->options & RES_DEBUG;
This addition setting the debug flag needs a bit of explaining in the
log message, me thinks. Why was it necessary or why is it better to do
it here?
Right now, the debug flag gets set in several places throughout the
code. Given you set the debug flag above, doesn't that mean several
code snippets setting the debug flag later in the code can go away?
Thanks,
Corinna
> I pushed patches 1 and 3 to 5. I fixed the consitency typo > throughout. Thanks! (and oops :-) > Right now, the debug flag gets set in several places throughout the > code. Given you set the debug flag above, doesn't that mean several > code snippets setting the debug flag later in the code can go away? No, they can't. The flag can be propagated from "res_init()" from the user land. When /etc/resolv.conf gets loaded, its "options" can also specify the debug setting (so it should become active since then), but formerly the code was using only the init-provided value in "get_resolv()" yet the debug setting from "options" (parsed by "get_options()") only affected the options themselves, but not the calling code in "get_resolv()", which kept on using the initial value. That made the remainder of the file parse to continue "silent" unless "res_init()" was previously called with RES_DEBUG. So that was, again, inconsistent! (see, I can spell it this time around :-) Post-"get_options()" assignment is not an additional assignment, it's a refresh of a possibly changed value (for a local "debug" variable). I think the patch is correct, and it works, for what I am concerned, -- I checked that and was using it. Anton Lavrentiev Contractor NIH/NLM/NCBI
On Jan 18 13:58, Lavrentiev, Anton (NIH/NLM/NCBI) [C] via Cygwin-patches wrote:
> > I pushed patches 1 and 3 to 5. I fixed the consitency typo
> > throughout.
>
> Thanks! (and oops :-)
>
> > Right now, the debug flag gets set in several places throughout the
> > code. Given you set the debug flag above, doesn't that mean several
> > code snippets setting the debug flag later in the code can go away?
>
> No, they can't. The flag can be propagated from "res_init()" from the user
> land. When /etc/resolv.conf gets loaded, its "options" can also specify the
> debug setting (so it should become active since then), but formerly the code was
> using only the init-provided value in "get_resolv()" yet the debug setting from
> "options" (parsed by "get_options()") only affected the options themselves,
> but not the calling code in "get_resolv()", which kept on using the initial value.
> That made the remainder of the file parse to continue "silent" unless "res_init()"
> was previously called with RES_DEBUG.
>
> So that was, again, inconsistent! (see, I can spell it this time around :-)
>
> Post-"get_options()" assignment is not an additional assignment, it's a refresh
> of a possibly changed value (for a local "debug" variable). I think the patch is correct,
> and it works, for what I am concerned, -- I checked that and was using it.
Thanks for the description. Would you mind to recreate your patch with
a matching commit message text explaining the debug flag setting?
Thanks,
Corinna