public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
* Cygwin: A few fixes for local resolver
@ 2022-01-14 22:10 Anton Lavrentiev
  2022-01-14 22:10 ` [PATCH 1/7] Fix format specifier for wide-char string Anton Lavrentiev
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Anton Lavrentiev @ 2022-01-14 22:10 UTC (permalink / raw)
  To: cygwin-patches

I am sending this patch that has fallen through the cracks almost a year ago, because I switched to a different
project and it was forgotten.  I just discovered it now, and I also added the AAAA record processing as discussed
back on Feb 1, 2021.

- Use %S (instead of %s) when a wide-character output is due;
- Use native byte order for host and add port when doing I/O with DNS server;
- Use forward way for resolv.conf's "options" processing, so listing "debug" as a
  first option, will show all following option(s) as they are read;
- Re-evaluate debug output flag after each "options" processing as it may chance.
- Added processing of AAAA records



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/7] Fix format specifier for wide-char string
  2022-01-14 22:10 Cygwin: A few fixes for local resolver Anton Lavrentiev
@ 2022-01-14 22:10 ` Anton Lavrentiev
  2022-01-14 22:10 ` [PATCH 2/7] Use matching format for NTSTATUS Anton Lavrentiev
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Anton Lavrentiev @ 2022-01-14 22:10 UTC (permalink / raw)
  To: cygwin-patches

---
 winsup/cygwin/libc/minires-os-if.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/winsup/cygwin/libc/minires-os-if.c b/winsup/cygwin/libc/minires-os-if.c
index 565158150..666a008de 100644
--- a/winsup/cygwin/libc/minires-os-if.c
+++ b/winsup/cygwin/libc/minires-os-if.c
@@ -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 %s\n", keyName);
+  DPRINTF(statp->options & RES_DEBUG, "key %ls\n", keyName);
   status = RtlCheckRegistryKey (RTL_REGISTRY_SERVICES, keyName);
   if (!NT_SUCCESS (status))
     {
-- 
2.33.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/7] Use matching format for NTSTATUS
  2022-01-14 22:10 Cygwin: A few fixes for local resolver Anton Lavrentiev
  2022-01-14 22:10 ` [PATCH 1/7] Fix format specifier for wide-char string Anton Lavrentiev
@ 2022-01-14 22:10 ` Anton Lavrentiev
  2022-01-15  4:38   ` Brian Inglis
  2022-01-14 22:10 ` [PATCH 3/7] Debug output to show both IP and port # in native b.o., a few little cosmetic improvements for consistency Anton Lavrentiev
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Anton Lavrentiev @ 2022-01-14 22:10 UTC (permalink / raw)
  To: cygwin-patches

---
 winsup/cygwin/libc/minires-os-if.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/libc/minires-os-if.c b/winsup/cygwin/libc/minires-os-if.c
index 666a008de..6e17de0b8 100644
--- a/winsup/cygwin/libc/minires-os-if.c
+++ b/winsup/cygwin/libc/minires-os-if.c
@@ -359,7 +359,7 @@ static void get_registry_dns(res_state statp)
   status = RtlCheckRegistryKey (RTL_REGISTRY_SERVICES, keyName);
   if (!NT_SUCCESS (status))
     {
-      DPRINTF (statp->options & RES_DEBUG, "RtlCheckRegistryKey: status %p\n",
+      DPRINTF (statp->options & RES_DEBUG, "RtlCheckRegistryKey: status 0x%08X\n",
 	       status);
       return;
     }
@@ -381,7 +381,7 @@ static void get_registry_dns(res_state statp)
   if (!NT_SUCCESS (status))
     {
       DPRINTF (statp->options & RES_DEBUG,
-	       "RtlQueryRegistryValues: status %p\n", status);
+	       "RtlQueryRegistryValues: status 0x%08X\n", status);
       return;
     }
 
-- 
2.33.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 3/7] Debug output to show both IP and port # in native b.o., a few little cosmetic improvements for consistency
  2022-01-14 22:10 Cygwin: A few fixes for local resolver Anton Lavrentiev
  2022-01-14 22:10 ` [PATCH 1/7] Fix format specifier for wide-char string Anton Lavrentiev
  2022-01-14 22:10 ` [PATCH 2/7] Use matching format for NTSTATUS Anton Lavrentiev
@ 2022-01-14 22:10 ` Anton Lavrentiev
  2022-01-17 10:39   ` Corinna Vinschen
  2022-01-14 22:10 ` [PATCH 4/7] Process options forward (not backwards) Anton Lavrentiev
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Anton Lavrentiev @ 2022-01-14 22:10 UTC (permalink / raw)
  To: cygwin-patches

---
 winsup/cygwin/libc/minires.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/winsup/cygwin/libc/minires.c b/winsup/cygwin/libc/minires.c
index 0979daae3..58c0438d3 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
@@ -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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 4/7] Process options forward (not backwards)
  2022-01-14 22:10 Cygwin: A few fixes for local resolver Anton Lavrentiev
                   ` (2 preceding siblings ...)
  2022-01-14 22:10 ` [PATCH 3/7] Debug output to show both IP and port # in native b.o., a few little cosmetic improvements for consistency Anton Lavrentiev
@ 2022-01-14 22:10 ` Anton Lavrentiev
  2022-01-14 22:10 ` [PATCH 5/7] Format consitency for Windows errors Anton Lavrentiev
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Anton Lavrentiev @ 2022-01-14 22:10 UTC (permalink / raw)
  To: cygwin-patches

---
 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 58c0438d3..2a77098b8 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 5/7] Format consitency for Windows errors
  2022-01-14 22:10 Cygwin: A few fixes for local resolver Anton Lavrentiev
                   ` (3 preceding siblings ...)
  2022-01-14 22:10 ` [PATCH 4/7] Process options forward (not backwards) Anton Lavrentiev
@ 2022-01-14 22:10 ` Anton Lavrentiev
  2022-01-14 22:10 ` [PATCH 6/7] Message consistency Anton Lavrentiev
  2022-01-14 22:10 ` [PATCH 7/7] Added processing of AAAA records Anton Lavrentiev
  6 siblings, 0 replies; 14+ messages in thread
From: Anton Lavrentiev @ 2022-01-14 22:10 UTC (permalink / raw)
  To: cygwin-patches

---
 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 6/7] Message consistency
  2022-01-14 22:10 Cygwin: A few fixes for local resolver Anton Lavrentiev
                   ` (4 preceding siblings ...)
  2022-01-14 22:10 ` [PATCH 5/7] Format consitency for Windows errors Anton Lavrentiev
@ 2022-01-14 22:10 ` Anton Lavrentiev
  2022-01-14 22:10 ` [PATCH 7/7] Added processing of AAAA records Anton Lavrentiev
  6 siblings, 0 replies; 14+ messages in thread
From: Anton Lavrentiev @ 2022-01-14 22:10 UTC (permalink / raw)
  To: cygwin-patches

---
 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 7/7] Added processing of AAAA records
  2022-01-14 22:10 Cygwin: A few fixes for local resolver Anton Lavrentiev
                   ` (5 preceding siblings ...)
  2022-01-14 22:10 ` [PATCH 6/7] Message consistency Anton Lavrentiev
@ 2022-01-14 22:10 ` Anton Lavrentiev
  6 siblings, 0 replies; 14+ messages in thread
From: Anton Lavrentiev @ 2022-01-14 22:10 UTC (permalink / raw)
  To: cygwin-patches

---
 winsup/cygwin/libc/minires-os-if.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/winsup/cygwin/libc/minires-os-if.c b/winsup/cygwin/libc/minires-os-if.c
index 6b4c5e33e..fd2e37a31 100644
--- a/winsup/cygwin/libc/minires-os-if.c
+++ b/winsup/cygwin/libc/minires-os-if.c
@@ -69,15 +69,15 @@ 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];
+    int i, sz = rr->wType == DNS_TYPE_A ? NS_INADDRSZ/*4*/ : NS_IN6ADDRSZ/*16*/;
+    if (ptr + sz <= EndPtr) {
+      for (i = 0;  i < sz;  ++i)
+        ptr[i] = aptr[i];
     }
-    ptr += 4;
+    ptr += sz;
     break;
   }
   case DNS_TYPE_NS:
-- 
2.33.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/7] Use matching format for NTSTATUS
  2022-01-14 22:10 ` [PATCH 2/7] Use matching format for NTSTATUS Anton Lavrentiev
@ 2022-01-15  4:38   ` Brian Inglis
  2022-01-15 19:04     ` [EXTERNAL] " Lavrentiev, Anton (NIH/NLM/NCBI) [C]
  0 siblings, 1 reply; 14+ messages in thread
From: Brian Inglis @ 2022-01-15  4:38 UTC (permalink / raw)
  To: cygwin-patches

See fprintf(3p) POSIX:
#   Specifies that the value is to be converted to an alternative form.
...
     For x or X  conversion  specifiers, a non-zero result shall have 0x 
(or 0X) prefixed to it.

On 2022-01-14 15:10, Anton Lavrentiev via Cygwin-patches wrote:
> ---
>   winsup/cygwin/libc/minires-os-if.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/winsup/cygwin/libc/minires-os-if.c b/winsup/cygwin/libc/minires-os-if.c
> index 666a008de..6e17de0b8 100644
> --- a/winsup/cygwin/libc/minires-os-if.c
> +++ b/winsup/cygwin/libc/minires-os-if.c
> @@ -359,7 +359,7 @@ static void get_registry_dns(res_state statp)
>     status = RtlCheckRegistryKey (RTL_REGISTRY_SERVICES, keyName);
>     if (!NT_SUCCESS (status))
>       {
> -      DPRINTF (statp->options & RES_DEBUG, "RtlCheckRegistryKey: status %p\n",
> +      DPRINTF (statp->options & RES_DEBUG, "RtlCheckRegistryKey: status 0x%08X\n",
          DPRINTF (statp->options & RES_DEBUG, "RtlCheckRegistryKey: 
status %#08x\n",
>   	       status);
>         return;
>       }
> @@ -381,7 +381,7 @@ static void get_registry_dns(res_state statp)
>     if (!NT_SUCCESS (status))
>       {
>         DPRINTF (statp->options & RES_DEBUG,
> -	       "RtlQueryRegistryValues: status %p\n", status);
> +	       "RtlQueryRegistryValues: status 0x%08x\n", status);
    	       "RtlQueryRegistryValues: status %#08x\n", status);
>         return;
>       }

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.
[Data in binary units and prefixes, physical quantities in SI.]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [EXTERNAL] Re: [PATCH 2/7] Use matching format for NTSTATUS
  2022-01-15  4:38   ` Brian Inglis
@ 2022-01-15 19:04     ` Lavrentiev, Anton (NIH/NLM/NCBI) [C]
  2022-01-15 22:38       ` Brian Inglis
  0 siblings, 1 reply; 14+ messages in thread
From: Lavrentiev, Anton (NIH/NLM/NCBI) [C] @ 2022-01-15 19:04 UTC (permalink / raw)
  To: Brian Inglis; +Cc: cygwin-patches

So?  With %X (capital X) the alternate form has the prefix 0X capital, too; and it's really hard to read.

IDK what is exactly your point that you are trying to make, is my patch somehow incorrect, or what?

Anton Lavrentiev
Contractor NIH/NLM/NCBI

> -----Original Message-----
> From: Brian Inglis <Brian.Inglis@SystematicSw.ab.ca>
> Sent: Friday, January 14, 2022 11:38 PM
> To: cygwin-patches@cygwin.com
> Subject: [EXTERNAL] Re: [PATCH 2/7] Use matching format for NTSTATUS
> 
> CAUTION: This email originated from outside of the organization. Do not click links or
> open attachments unless you recognize the sender and are confident the content is safe.
> 
> 
> See fprintf(3p) POSIX:
> #   Specifies that the value is to be converted to an alternative form.
> ...
>      For x or X  conversion  specifiers, a non-zero result shall have 0x
> (or 0X) prefixed to it.
> 
> On 2022-01-14 15:10, Anton Lavrentiev via Cygwin-patches wrote:
> > ---
> >   winsup/cygwin/libc/minires-os-if.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/winsup/cygwin/libc/minires-os-if.c b/winsup/cygwin/libc/minires-os-if.c
> > index 666a008de..6e17de0b8 100644
> > --- a/winsup/cygwin/libc/minires-os-if.c
> > +++ b/winsup/cygwin/libc/minires-os-if.c
> > @@ -359,7 +359,7 @@ static void get_registry_dns(res_state statp)
> >     status = RtlCheckRegistryKey (RTL_REGISTRY_SERVICES, keyName);
> >     if (!NT_SUCCESS (status))
> >       {
> > -      DPRINTF (statp->options & RES_DEBUG, "RtlCheckRegistryKey: status %p\n",
> > +      DPRINTF (statp->options & RES_DEBUG, "RtlCheckRegistryKey: status 0x%08X\n",
>           DPRINTF (statp->options & RES_DEBUG, "RtlCheckRegistryKey:
> status %#08x\n",
> >              status);
> >         return;
> >       }
> > @@ -381,7 +381,7 @@ static void get_registry_dns(res_state statp)
> >     if (!NT_SUCCESS (status))
> >       {
> >         DPRINTF (statp->options & RES_DEBUG,
> > -            "RtlQueryRegistryValues: status %p\n", status);
> > +            "RtlQueryRegistryValues: status 0x%08x\n", status);
>                "RtlQueryRegistryValues: status %#08x\n", status);
> >         return;
> >       }
> 
> --
> Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada
> 
> This email may be disturbing to some readers as it contains
> too much technical detail. Reader discretion is advised.
> [Data in binary units and prefixes, physical quantities in SI.]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [EXTERNAL] Re: [PATCH 2/7] Use matching format for NTSTATUS
  2022-01-15 19:04     ` [EXTERNAL] " Lavrentiev, Anton (NIH/NLM/NCBI) [C]
@ 2022-01-15 22:38       ` Brian Inglis
  2022-01-16  0:20         ` Lavrentiev, Anton (NIH/NLM/NCBI) [C]
  0 siblings, 1 reply; 14+ messages in thread
From: Brian Inglis @ 2022-01-15 22:38 UTC (permalink / raw)
  To: cygwin-patches

Just the suggestion that as all standards support using %#08x to prefix 
with 0x (prefix output capitalization follows format letter 
capitalization) and would be preferable to hacking the text 0x onto the 
format %08X, doing all of the formatting work with the format flags.

My awareness and attitude to modifying output presentation using only 
formats was hardened by those not using date formats to modify date 
presentation during projects prior to Y2K!

[I want to scream and rant when I see imbeciles still producing output 
using meaningless 10/11/12 date formats, on systems and especially on 
web sites, where JavaScript supports perfectly nice internationalized 
formatting that shows dates and times in my zone and preferred formats!]

On 2022-01-15 12:04, Lavrentiev, Anton (NIH/NLM/NCBI) [C] wrote:
> So?  With %X (capital X) the alternate form has the prefix 0X capital, too; and it's really hard to read.
> 
> IDK what is exactly your point that you are trying to make, is my patch somehow incorrect, or what?
> 
> Anton Lavrentiev
> Contractor NIH/NLM/NCBI
> 
>> -----Original Message-----
>> From: Brian Inglis <Brian.Inglis@SystematicSw.ab.ca>
>> Sent: Friday, January 14, 2022 11:38 PM
>> To: cygwin-patches@cygwin.com
>> Subject: [EXTERNAL] Re: [PATCH 2/7] Use matching format for NTSTATUS
>>
>> CAUTION: This email originated from outside of the organization. Do not click links or
>> open attachments unless you recognize the sender and are confident the content is safe.
>>
>>
>> See fprintf(3p) POSIX:
>> #   Specifies that the value is to be converted to an alternative form.
>> ...
>>       For x or X  conversion  specifiers, a non-zero result shall have 0x
>> (or 0X) prefixed to it.
>>
>> On 2022-01-14 15:10, Anton Lavrentiev via Cygwin-patches wrote:
>>> ---
>>>    winsup/cygwin/libc/minires-os-if.c | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/winsup/cygwin/libc/minires-os-if.c b/winsup/cygwin/libc/minires-os-if.c
>>> index 666a008de..6e17de0b8 100644
>>> --- a/winsup/cygwin/libc/minires-os-if.c
>>> +++ b/winsup/cygwin/libc/minires-os-if.c
>>> @@ -359,7 +359,7 @@ static void get_registry_dns(res_state statp)
>>>      status = RtlCheckRegistryKey (RTL_REGISTRY_SERVICES, keyName);
>>>      if (!NT_SUCCESS (status))
>>>        {
>>> -      DPRINTF (statp->options & RES_DEBUG, "RtlCheckRegistryKey: status %p\n",
>>> +      DPRINTF (statp->options & RES_DEBUG, "RtlCheckRegistryKey: status 0x%08X\n",
>>            DPRINTF (statp->options & RES_DEBUG, "RtlCheckRegistryKey:
>> status %#08x\n",
>>>               status);
>>>          return;
>>>        }
>>> @@ -381,7 +381,7 @@ static void get_registry_dns(res_state statp)
>>>      if (!NT_SUCCESS (status))
>>>        {
>>>          DPRINTF (statp->options & RES_DEBUG,
>>> -            "RtlQueryRegistryValues: status %p\n", status);
>>> +            "RtlQueryRegistryValues: status 0x%08x\n", status);
>>                 "RtlQueryRegistryValues: status %#08x\n", status);
>>>          return;
>>>        }

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.
[Data in binary units and prefixes, physical quantities in SI.]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [EXTERNAL] Re: [PATCH 2/7] Use matching format for NTSTATUS
  2022-01-15 22:38       ` Brian Inglis
@ 2022-01-16  0:20         ` Lavrentiev, Anton (NIH/NLM/NCBI) [C]
  0 siblings, 0 replies; 14+ messages in thread
From: Lavrentiev, Anton (NIH/NLM/NCBI) [C] @ 2022-01-16  0:20 UTC (permalink / raw)
  To: cygwin-patches

> Just the suggestion that as all standards support using %#08x to prefix
> with 0x (prefix output capitalization follows format letter
> capitalization) and would be preferable to hacking the text 0x onto the
> format %08X, doing all of the formatting work with the format flags.

First off, I am perfectly aware of the "alternate" form, and I don't like it
because it either creates all-caps representation (which I find hard to read,
and I mentioned that), or it creates a "camel-case" representation (all "normal"
digits are "capital" letters, while a-f are not, and I do not like that,
either).

> My awareness and attitude to modifying output presentation using only
> formats was hardened by those not using date formats to modify date
> presentation during projects prior to Y2K!

It may be so but it has nothing to do with the output of NT status here.

Also, a little tolerance of how something can be done not exactly the way
you would do it, goes a long way.

Thank you for not trolling,

Anton Lavrentiev
Contractor NIH/NLM/NCBI


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/7] Debug output to show both IP and port # in native b.o., a few little cosmetic improvements for consistency
  2022-01-14 22:10 ` [PATCH 3/7] Debug output to show both IP and port # in native b.o., a few little cosmetic improvements for consistency Anton Lavrentiev
@ 2022-01-17 10:39   ` Corinna Vinschen
  2022-01-17 10:44     ` Corinna Vinschen
  0 siblings, 1 reply; 14+ messages in thread
From: Corinna Vinschen @ 2022-01-17 10:39 UTC (permalink / raw)
  To: cygwin-patches

Hi Anton,

I pushed the first two patches.  However...

On Jan 14 17:10, Anton Lavrentiev via Cygwin-patches wrote:
> ---
>  winsup/cygwin/libc/minires.c | 28 ++++++++++++++++++----------
>  1 file changed, 18 insertions(+), 10 deletions(-)

...would you mind to shorten your commit caption (aka subject), at least
to less than 80 chars, and outline your patch in the remainder of the
commit message, please?

In fact, a short subject and a bit of explaining text in the commit
message is preferrable in general.  See the DISCUSSION section in the
git-commit(1) man page.


Thanks,
Corinna

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/7] Debug output to show both IP and port # in native b.o., a few little cosmetic improvements for consistency
  2022-01-17 10:39   ` Corinna Vinschen
@ 2022-01-17 10:44     ` Corinna Vinschen
  0 siblings, 0 replies; 14+ messages in thread
From: Corinna Vinschen @ 2022-01-17 10:44 UTC (permalink / raw)
  To: cygwin-patches

On Jan 17 11:39, Corinna Vinschen wrote:
> Hi Anton,
> 
> I pushed the first two patches.  However...
> 
> On Jan 14 17:10, Anton Lavrentiev via Cygwin-patches wrote:
> > ---
> >  winsup/cygwin/libc/minires.c | 28 ++++++++++++++++++----------
> >  1 file changed, 18 insertions(+), 10 deletions(-)
> 
> ...would you mind to shorten your commit caption (aka subject), at least
> to less than 80 chars, and outline your patch in the remainder of the
> commit message, please?
> 
> In fact, a short subject and a bit of explaining text in the commit
> message is preferrable in general.  See the DISCUSSION section in the
> git-commit(1) man page.

Oh, and, while we're at it, please add a prefix to the subjects of your
remaining patches, i .e.

  Cygwin: resolver: blah

I forget that for the first two patches, but we should do this a bit
more thorough.

Other than that, the remaining patches look good, except, adding a short
description what patch 7 does to the commit message would be great for
later readers of the git log.


Thanks,
Corinna

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-01-17 10:44 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-14 22:10 Cygwin: A few fixes for local resolver Anton Lavrentiev
2022-01-14 22:10 ` [PATCH 1/7] Fix format specifier for wide-char string Anton Lavrentiev
2022-01-14 22:10 ` [PATCH 2/7] Use matching format for NTSTATUS Anton Lavrentiev
2022-01-15  4:38   ` Brian Inglis
2022-01-15 19:04     ` [EXTERNAL] " Lavrentiev, Anton (NIH/NLM/NCBI) [C]
2022-01-15 22:38       ` Brian Inglis
2022-01-16  0:20         ` Lavrentiev, Anton (NIH/NLM/NCBI) [C]
2022-01-14 22:10 ` [PATCH 3/7] Debug output to show both IP and port # in native b.o., a few little cosmetic improvements for consistency Anton Lavrentiev
2022-01-17 10:39   ` Corinna Vinschen
2022-01-17 10:44     ` Corinna Vinschen
2022-01-14 22:10 ` [PATCH 4/7] Process options forward (not backwards) Anton Lavrentiev
2022-01-14 22:10 ` [PATCH 5/7] Format consitency for Windows errors Anton Lavrentiev
2022-01-14 22:10 ` [PATCH 6/7] Message consistency Anton Lavrentiev
2022-01-14 22:10 ` [PATCH 7/7] Added processing of AAAA records Anton Lavrentiev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).