RE: [EXTERNAL] Re: [PATCH] Cygwin: spawn: Treat empty path as the current directory.
>However, use of this feature is deprecated, and POSIX >notes that a conforming application shall use an explicit >pathname (e.g., .) to specify the current working >directory. Since "SHALL" does not mean "MUST", I think this patch is correct. Anton Lavrentiev Contractor NIH/NLM/NCBI
RE: [PATCH] Cygwin: resolver: A few fixes for cygwin_query(), part 2
> Great, thanks! I pushed all your resolver patches. Thank you! I'll be submitting the last tiny little one in a sec :-) Anton Lavrentiev Contractor NIH/NLM/NCBI
RE: [PATCH 2/5] Cygwin: resolver: Process options forward (not backwards)
> Thanks for the description. Would you mind to recreate your patch with > a matching commit message text explaining the debug flag setting? Okay, just did. Anton Lavrentiev Contractor NIH/NLM/NCBI
RE: [EXTERNAL] Re: [PATCH 2/5] Cygwin: resolver: Process options forward (not backwards)
> 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
RE: [EXTERNAL] Re: [PATCH] Cygwin: Conditionally build documentation
> It is reported by 'configure --help', at the appropriate level (although > since enable is the default, I probably should have written > '--disable-doc' here). Can you please make it show as --disable-doc ? Also, can you please make it visible in the top-level configure? Thanks, Anton Lavrentiev Contractor NIH/NLM/NCBI
RE: resolver //Was: [PATCH 3/7] Debug output to show both IP and port # in native b.o., a few little cosmetic improvements for consistency
Hi Corinna, > 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. I resubmitted the patches with a little improvement and a better description to the #7 (now #5) as requested. While doing the code review afresh in there, I noticed a few more problems: 1. minires-os-if.c on line 262 does this: 262 ptr = NULL; 263 break; and then a bit later this: 290 len = ptr - AnsPtr; which makes the return value "len" to be a total nonsense (I think it should return -1 in this case, but it's debatable). 2. Also, "ptr" in the cygwin_query() function is not checked for buffer overrun in the "done:" section of the code (after line 291), which is not good IMO. 3. Lastly, at other places where "ptr" is checked for overrun (notably, in write_record()), it can leave garbage in the unfilled portion of the answer buffer (because it still advances "ptr" properly, to calculate the final "would-be" size of the response): so if the return value is greater than the passed "AnsLength", the user cannot assume that at least all AnsLength bytes were filled correctly. They cannot actually assume any "boundary" where the "Ans" buffer was actually stopped being updated. Maybe "Ans" should be cleared upon entry?... But that would mean double-write of the buffer in most of the use-cases (where no overflow actually occurs because of an adequate size of the buffer). Anton Lavrentiev Contractor NIH/NLM/NCBI
RE: [EXTERNAL] Re: [PATCH 2/7] Use matching format for NTSTATUS
> 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
RE: [EXTERNAL] Re: [PATCH] Cygwin: Conditionally build documentation
> It is reported by 'configure --help', at the appropriate level (although > since enable is the default, I probably should have written > '--disable-doc' here). I'm sorry if I'm missing anything, but I updated y'day and this is what I see regarding the doc in configure: $ ./configure --help | grep doc --infodir=DIR info documentation [DATAROOTDIR/info] --mandir=DIRman documentation [DATAROOTDIR/man] --docdir=DIRdocumentation root [DATAROOTDIR/doc/PACKAGE] --htmldir=DIR html documentation [DOCDIR] --dvidir=DIRdvi documentation [DOCDIR] --pdfdir=DIRpdf documentation [DOCDIR] --psdir=DIR ps documentation [DOCDIR] Anton Lavrentiev Contractor NIH/NLM/NCBI
RE: [EXTERNAL] Re: [PATCH 2/7] Use matching format for NTSTATUS
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
> 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.]
RE: [PATCH] Cygwin: Conditionally build documentation
> > Add a configure option '--disable-doc' to disable building of the > documentation by the 'all' target. > Can you please also add --disable-doc to "configure --help"? It took me awhile to figure out which option I should use to skip the doc from building because it does no longer ignore doc build failure by default (unlike it used to do). Also this fact is not reflected in the FAQ here: https://cygwin.com/faq.html#faq.programming.building-cygwin which still mentions the doc build errors ignored: > Normally, building ignores any errors in building the documentation Thanks! Anton Lavrentiev Contractor NIH/NLM/NCBI
RE: Fix nanosleep returning negative rem
> I can get it easily get this on my desktop (AMD Ryzen Threadripper 3990X) but
> not at all on my laptop (Intel Core i7-8650U)
Not sure if that's really related but:
Have you checked what is your default Windows timer resolution? Some
applications change it from the default
100HZ to 1000HZ (by e.g. using timeBeginPeriod(1) -- MS Edge does this, and
Chrome, I think, but not Firefox -- yet
the change is actually _global_ as documented).
The following code shows the timer resolution:
#include
#include
#include
#include
int main()
{
unsigned int prev = timeGetTime(), next;
Sleep(1);
next = timeGetTime();
printf("Sleep(1) = %u\n", next - prev);
return 0;
}
Output:
Sleep(1) = 10
means the default 100HZ
Sleep(1) = 1
means the increased resolution of 1ms
Anton Lavrentiev
Contractor NIH/NLM/NCBI
P.S. timeBeginPeriod():
This function affects a global Windows setting. Windows uses the lowest value
(that is, highest
resolution) requested by any process. Setting a higher resolution can improve
the accuracy of
time-out intervals in wait functions. However, it can also reduce overall
system performance,
because the thread scheduler switches tasks more often. High resolutions can
also prevent the
CPU power management system from entering power-saving modes.
RE: [PATCH] CYGWIN: Fix resolver debugging output
> with some customizations, and not noticed any issues so far. Should I be wary > in future? Have you used the / APIs, or low-level DNS other than with getXbyY(), or maybe debugged DNS? The changes are not to remove any existing functionality -- I don't see how that was not clear -- but to make it easier to use the API when you have to do a little extra, like seeing the nameserver IP addresses in a more human-readable form. But TBH, using native API (in the absence of /etc/resolv.conf, or by using "options osquery" in it -- but then most of the stuff in that file is just simply ignored, JFYI), is the best way of dealing with DNS in CYGWIN. The minires.c implementation is rather simplistic (but it's good when one needs to stay in full control / observe of what they are doing -- and for me is the development stage, but I'll drop that and switch back to the OS API once everything is working as it should, for the project that I'm dealing with right now). Also, there was at least one bug.
RE: [PATCH] CYGWIN: Fix resolver debugging output
> Except, the value has no meaning for ipv6. It'll print all 0's :-) But: minires does not make use of the _ext field. It does use the conventional nsaddr_list (which is IPv4), but only if Windows native DNS API is not used: "osquery"(aka use_os)=0. For debugging purposes, that is enough and very convenient (yet the output needed some tune-up, which I suggested in my patch). But for practical purposes, only Windows API should be used in regular applications (which is the default, anyways, since /etc/resolv.conf is not routinely provided, so "osquery=1" implicitly). I'm not sure if improvements to use _ext by the minires own code would be any beneficial. Having said that, replies should be made understood by the minires-if-os shim code (and I can provide a patch for that, too).
RE: [PATCH] CYGWIN: Fix resolver debugging output
> Please use %ls, %S is non-standard.
Sure.
> For instance, write_record appears to handle DNS_TYPE_A,
> but not DNS_TYPE_.
I can add that, it's not a problem. But indeed, reparsing of Windows packets,
does miss (as well as some other types, such as URI -- not sure if Windows
has it, though).
> Would you mind to split this into a patchset with patches for different
> tasks? ATM I'm a bit concerned about the ntoh{sl} calls, given the
> noticable absence of IPv6 support...
Okay. BTW, I added ntol/s only for output of *nameserver*'s IPv4:port, because
nameservers are IPv4 (even in glibc, AFAIK). The _res structure (same in glibc)
has these addresses as "struct in_addr", meaning they are IPv4. And so there's
no risk of running into any troubles, but reading the IP addresses in debug
output
is much easier if they are in native order (and same goes for ports, even more).
RE: [PATCH] format_proc_cpuinfo: add microcode registry lookup values
Hi,
This is shifting up, IMO:
+ microcode <<= 32; /* shift them down */
Thanks,
Anton
> -Original Message-
> From: Brian Inglis
> Sent: Tuesday, July 07, 2020 1:34 PM
> To: cygwin-patches@cygwin.com
> Subject: [PATCH] format_proc_cpuinfo: add microcode registry lookup values
>
> Re: CPU microcode reported wrong in /proc/cpuinfo
> https://sourceware.org/pipermail/cygwin/2020-May/245063.html
> earlier Windows releases used different registry values to store microcode
> revisions depending on the MSR name being used to get microcode revisions:
> add these alternative registry values to the cpuinfo registry value lookup;
> iterate thru the registry data until a valid microcode revision is found;
> some revision values are in the high bits, so if the low bits are all clear,
> shift the revision value down into the low bits
> ---
> winsup/cygwin/fhandler_proc.cc | 44 +++---
> 1 file changed, 35 insertions(+), 9 deletions(-)
>
> diff --git a/winsup/cygwin/fhandler_proc.cc b/winsup/cygwin/fhandler_proc.cc
> index f1bc1c7405..f637dfd8e4 100644
> --- a/winsup/cygwin/fhandler_proc.cc
> +++ b/winsup/cygwin/fhandler_proc.cc
> @@ -692,26 +692,52 @@ format_proc_cpuinfo (void *, char *&destbuf)
>union
> {
> LONG uc_len; /* -max size of buffer before call */
> - char uc_microcode[16];
> -} uc;
> + char uc_microcode[16];/* at least 8 bytes */
> +} uc[4]; /* microcode values changed historically */
>
> - RTL_QUERY_REGISTRY_TABLE tab[3] =
> + RTL_QUERY_REGISTRY_TABLE tab[6] =
> {
> { NULL, RTL_QUERY_REGISTRY_DIRECT | RTL_QUERY_REGISTRY_NOSTRING,
> - L"~Mhz", &cpu_mhz, REG_NONE, NULL, 0 },
> + L"~Mhz", &cpu_mhz, REG_NONE, NULL, 0 },
> { NULL, RTL_QUERY_REGISTRY_DIRECT | RTL_QUERY_REGISTRY_NOSTRING,
> - L"Update Revision", &uc, REG_NONE, NULL, 0 },
> + L"Update Revision", &uc[0], REG_NONE, NULL, 0 },
> + /* latest MSR */
> + { NULL, RTL_QUERY_REGISTRY_DIRECT | RTL_QUERY_REGISTRY_NOSTRING,
> + L"Update Signature", &uc[1], REG_NONE, NULL, 0 },
> + /* previous MSR */
> + { NULL, RTL_QUERY_REGISTRY_DIRECT | RTL_QUERY_REGISTRY_NOSTRING,
> + L"CurrentPatchLevel",&uc[2], REG_NONE, NULL, 0 },
> + /* earlier MSR */
> + { NULL, RTL_QUERY_REGISTRY_DIRECT | RTL_QUERY_REGISTRY_NOSTRING,
> + L"Platform Specific Field1", &uc[3], REG_NONE, NULL, 0 },
> + /* alternative */
> { NULL, 0, NULL, NULL, 0, NULL, 0 }
> };
>
> - memset (&uc, 0, sizeof (uc.uc_microcode));
> - uc.uc_len = -16; /* -max size of microcode buffer */
> + for (size_t uci = 0; uci < sizeof (uc)/sizeof (*uc); ++uci)
> + {
> + memset (&uc[uci], 0, sizeof (uc[uci]));
> + uc[uci].uc_len = -(LONG)sizeof (uc[0].uc_microcode);
> + /* neg buffer size */
> + }
> +
>RtlQueryRegistryValues (RTL_REGISTRY_ABSOLUTE, cpu_key, tab,
> NULL, NULL);
>cpu_mhz = ((cpu_mhz - 1) / 10 + 1) * 10; /* round up to multiple
> of 10 */
>DWORD bogomips = cpu_mhz * 2; /* bogomips is double cpu MHz since MMX
> */
> - long long microcode = 0; /* at least 8 bytes for AMD */
> - memcpy (µcode, &uc, sizeof (microcode));
> +
> + unsigned long long microcode = 0; /* needs 8 bytes */
> + for (size_t uci = 0; uci < sizeof (uc)/sizeof (*uc) && !microcode;
> ++uci)
> + {
> + /* still neg buffer size => no data */
> + if (-(LONG)sizeof (uc[uci].uc_microcode) != uc[uci].uc_len)
> + {
> + memcpy (µcode, uc[uci].uc_microcode, sizeof (microcode));
> +
> + if (!(microcode & 0xLL)) /* some values in high bits */
> + microcode <<= 32; /* shift them down */
> + }
> + }
>
>bufptr += __small_sprintf (bufptr, "processor\t: %d\n", cpu_number);
>uint32_t maxf, vendor_id[4], unused;
> --
> 2.27.0
RE: [PATCH] Cygwin: console: Ignore 0x00 on write().
JFYI:
Both 0x00 (NUL) and 0x7F (DEL) used to be the filler characters, and were
ignored by most (hardware) terminals from the very early days.
HTH,
Anton
> -Original Message-
> From: cygwin-patches-ow...@cygwin.com On
> Behalf Of Takashi Yano
> Sent: Thursday, February 20, 2020 6:52 AM
> To: cygwin-patches@cygwin.com
> Cc: Takashi Yano
> Subject: [PATCH] Cygwin: console: Ignore 0x00 on write().
>
> - In xterm compatible mode, 0x00 on write() behaves incompatible
> with real xterm. In xterm, 0x00 completely ignored. Therefore,
> 0x00 is ignored by console with this patch.
> ---
> winsup/cygwin/fhandler_console.cc | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/winsup/cygwin/fhandler_console.cc
> b/winsup/cygwin/fhandler_console.cc
> index 66e645aa1..705ce696e 100644
> --- a/winsup/cygwin/fhandler_console.cc
> +++ b/winsup/cygwin/fhandler_console.cc
> @@ -1794,6 +1794,16 @@ bool fhandler_console::write_console (PWCHAR buf,
> DWORD len, DWORD& done)
> len -= 4;
> }
> }
> + /* Workaround for ^@ (0x00) handling in xterm compatible mode. */
> + if (wincap.has_con_24bit_colors () && !con_is_legacy)
> +{
> + WCHAR *p = buf;
> + while ((p = wmemchr (p, L'\0', len - (p - buf
> + {
> + memmove (p, p+1, (len - (p+1 - buf))*sizeof (WCHAR));
> + len --;
> + }
> +}
>
>if (con.iso_2022_G1
> ? con.vt100_graphics_mode_G1
> --
> 2.21.0
RE: [PATCH] Cygwin: pty: Fix ESC[?3h and ESC[?3l handling again.
Just noticed that in the patch below and couldn't help it, sorry..
Things like
char* p0;
and later:
isdigit(*p0)) or isalpha(*p0)
are usually not a good (correct) way of coding, because of possible sign
extension of *p0
which you normally wouldn't want to have (remember the ctype calls/macros
actually expect
an "int", not a character, input). So it should be either "unsigned char* p0"
or
"isdigit((unsigned char)(*p0))", generally.
> -Original Message-
> From: cygwin-patches-ow...@cygwin.com On
> Behalf Of Corinna Vinschen
> Sent: Thursday, December 19, 2019 6:29 AM
> To: cygwin-patches@cygwin.com
> Subject: Re: [PATCH] Cygwin: pty: Fix ESC[?3h and ESC[?3l handling again.
>
> On Dec 19 20:03, Takashi Yano wrote:
> > - Even with commit fe512b2b12a2cea8393d14f038dc3914b1bf3f60, pty
> > still has a problem in ESC[?3h and ESC[?3l handling if invalid
> > sequence such as ESC[?$ is sent. This patch fixes the issue.
> > ---
> > winsup/cygwin/fhandler_tty.cc | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
> > index 8c3a6e72e..f10f0fc61 100644
> > --- a/winsup/cygwin/fhandler_tty.cc
> > +++ b/winsup/cygwin/fhandler_tty.cc
> > @@ -1263,7 +1263,7 @@ fhandler_pty_slave::push_to_pcon_screenbuffer (const
> char *ptr, size_t len)
> > {
> >p0 += 3;
> >bool exist_arg_3 = false;
> > - while (p0 < buf + nlen && !isalpha (*p0))
> > + while (p0 < buf + nlen && (isdigit (*p0) || *p0 == ';'))
> > {
> > int arg = 0;
> > while (p0 < buf + nlen && isdigit (*p0))
> > --
> > 2.21.0
>
> Pushed.
>
>
> Thanks,
> Corinna
>
> --
> Corinna Vinschen
> Cygwin Maintainer
RE: cygrunsrv patch
> git://sourceware.org/git/cygwin-apps/cygrunsrv.git Thanks, that worked. I was using a static source archive (cygrunsrv-1.62) to make all the changes, so didn't need git until now.
cygrunsrv patch
Hi, Looks like I finally prepared a patch for cygrunsrv that I mentioned as an added feature way back in Feb (I did not need to come around to that since then): https://cygwin.com/ml/cygwin/2019-02/msg00173.html But the GIT URL given in the message does not seem to work for me: https://cygwin.com/git/?p=cygwin-apps/cygrunsrv.git $ git clone 'https://cygwin.com/git/?p=cygwin-apps/cygrunsrv.git' ./cygrunsrv Cloning into './cygrunsrv'... fatal: repository 'https://cygwin.com/git/?p=cygwin-apps/cygrunsrv.git/' not found Any ideas? Thanks, Anton
RE: [PATCH] Cygwin: getpriority() consistent with process priority
> Pushed. Thank you! It took me awhile, sorry about that -- was extremely busy with stuff and travel
