public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
From: Ken Brown <kbrown@cornell.edu>
To: Jon Turney <jon.turney@dronecode.org.uk>,
	Cygwin Patches <cygwin-patches@cygwin.com>
Subject: Re: [PATCH] Cygwin: winlean.h: remove most of extended memory API
Date: Thu, 24 Sep 2020 12:36:50 -0400	[thread overview]
Message-ID: <5c0bf066-a052-b9fd-36d3-0b6805ab0257@cornell.edu> (raw)
In-Reply-To: <ddeace5b-33a2-ed1f-5b30-0d33bfe61ca3@dronecode.org.uk>

On 9/24/2020 10:04 AM, Jon Turney wrote:
> On 24/09/2020 00:52, Ken Brown via Cygwin-patches wrote:
>> This was added as a temporary measure in commit e18f7f99 because it
>> wasn't yet in the mingw-w64 headers.  With one exception, it is now in
>> the current release of the headers (version 8.0.0), so we don't need
>> it in winlean.h.  The exception is that VirtualAlloc2 is only declared
>> conditionally in <w32api/memoryapi.h>, so retain it in winlean.h.  Add
> 
> I assume it's conditional on the windows version targetted, but it might help to 
> mention that in a comment.

Done.

>> "WINAPI" to its declaration for consistency with the delaration in
>> memoryapi.h.
>>
>> Also revert commit 3d136011, which was a related temporary workaround.
> 
> Looks good to me.
> 
> I think this isn't going work any more with older win32api, but we probably 
> don't care about that.  It would perhaps be nice to explicitly complain about 
> that (checking __MINGW64_VERSION_MAJOR somehow), rather than exploding 
> incomprehensibly if the w32api is too old?

We probably don't care much, since anyone building Cygwin should have the latest 
tools installed.  On the other hand, it's simple to add that check, so I've done it.

>> In particular, I'd like to know if my handling of the declaration of 
>> VirtualAlloc2 seems reasonable.  Among other things, I'm puzzled by the 
>> apparent need to add WINAPI.  If it's really needed, I don't know how the 
>> calls of that function could have worked before.  Can anyone enlighten me?
> 
> I believe that WINAPI only does something (stdcall) on x86, so it might well be 
> that it's never worked on Windows 10 =>1803 x86?

OK, I feel better now.

Thanks for the review.

Ken

  reply	other threads:[~2020-09-24 16:37 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-23 23:52 Ken Brown
2020-09-24 14:04 ` Jon Turney
2020-09-24 16:36   ` Ken Brown [this message]
2020-10-13 11:00   ` Corinna Vinschen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5c0bf066-a052-b9fd-36d3-0b6805ab0257@cornell.edu \
    --to=kbrown@cornell.edu \
    --cc=cygwin-patches@cygwin.com \
    --cc=jon.turney@dronecode.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).