Re: [PATCH] Cygwin: Fix error return for madvise()

Hi Takashi,

On 7/6/2026 5:45 PM, Takashi Yano wrote:
> Hi Mark,
> 
> On Mon,  6 Jul 2026 16:47:43 -0700
> Mark Geisert wrote:
>> Currently madvise() and posix_madvise() are wired together as one
>> function: the latter.  But their error returns should be different.
>> Make madvise a first-class export in cygwin.din; code a new madvise()
>> that calls posix_madvise() and massages any error return.
>>
>> Reported-by: Christian Franke <[email protected]>
>> Addresses: https://cygwin.com/pipermail/cygwin/2026-July/259872.html
>> Signed-off-by: Mark Geisert <[email protected]>
>> Fixes: 61522196c715 (* Merge in cygwin-64bit-branch.)
>>
>> ---
>>   winsup/cygwin/cygwin.din               |  2 +-
>>   winsup/cygwin/include/cygwin/version.h |  3 ++-
>>   winsup/cygwin/mm/mmap.cc               | 12 ++++++++++++
>>   3 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/winsup/cygwin/cygwin.din b/winsup/cygwin/cygwin.din
>> index 2e53bc819..937eacdaf 100644
>> --- a/winsup/cygwin/cygwin.din
>> +++ b/winsup/cygwin/cygwin.din
>> @@ -951,7 +951,7 @@ lseek SIGFE
>>   lsetxattr SIGFE
>>   lstat SIGFE
>>   lutimes SIGFE
>> -madvise = posix_madvise SIGFE
>> +madvise SIGFE
>>   makecontext NOSIGFE
>>   mallinfo SIGFE
>>   malloc SIGFE
>> diff --git a/winsup/cygwin/include/cygwin/version.h b/winsup/cygwin/include/cygwin/version.h
>> index 71ac5282b..fc838e23e 100644
>> --- a/winsup/cygwin/include/cygwin/version.h
>> +++ b/winsup/cygwin/include/cygwin/version.h
>> @@ -502,12 +502,13 @@ details. */
>>     360: Add RLIMIT_NPROC.
>>     361: Export _Fork.
>>     362: Export C23 stdbit functions.
>> +  363: Export madvise separately from posix_madvise.
>>   
>>     Note that we forgot to bump the api for ualarm, strtoll, strtoull,
>>     sigaltstack, sethostname. */
>>   
>>   #define CYGWIN_VERSION_API_MAJOR 0
>> -#define CYGWIN_VERSION_API_MINOR 362
>> +#define CYGWIN_VERSION_API_MINOR 363
>>   
>>   /* There is also a compatibity version number associated with the shared memory
>>      regions.  It is incremented when incompatible changes are made to the shared
> 
> I don't think we should change CYGWIN_VERSION_API_MINOR value
> because the API itself is not changed. This patch fixes a bug
> in madvice() implementation.

I went back and forth internally on whether the minor version should be 
bumped.  One point was whether divorcing madvise() from posix_madvise() 
in cygwin.din warranted an API bump: without a bump won't existing 
programs be unable to access the new error return behavior?  Another 
point was that the error return behavior of madvise() is being changed; 
isn't that behavior part of the API?  I agree this is a bug fix but such 
fixes could cause API changes.

This is a case where I'd like to get input from other folks who might've 
made similar changes to Cygwin over its long history.  I am totally OK 
with removing the API bump in a v2 patch if that's the consensus opinion.
Thank you for the review!

..mark