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

Hi,

Takashi Yano wrote:
> Hi Mark,
>
> On Tue, 7 Jul 2026 00:58:48 -0700
> Mark Geisert wrote:
>> Hi Takashi,
>>
>> On 7/6/2026 5:45 PM, Takashi Yano wrote:
>>> ...
>>> 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?

Existing programs will call the new madvise() function. The existing 
DLLs provide a distinct symbol 'madvise' which will then arrive at the 
new function.


>>    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.
> I could not found the case that API version bump for behavioral change
> in the past.
>
> The adding MACROs such as:
>    148: Add open(2) flags O_SYNC, O_RSYNC, O_DSYNC and O_DIRECT.
>    149: Add open(2) flag O_NOFOLLOW.
> could be a 'behavioral change', but header files for user were also
> changed in these cases.

For this change of the return value of nice(), no API bump was requested:
https://sourceware.org/pipermail/cygwin-patches/2024q4/013015.html

 From the point of view of officially released versions, it would be 
possibly sufficient if the first 3.7.* release will have a larger minor 
API version than any 3.6.* has or will have.


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

Two unrelated suggestions for a possible v2 patch:

- Add a note to winsup/cygwin/release/3.7.0

- Provide distinct 'syscall_printf()' for both functions: rename 'extern 
... posix_madvise()' to e.g. `static ... posix_madvise_worker()`, add 
two new 'extern ... *madvise()' functions calling it, move the 
'syscall_printf()' (which then also allows to remove the 'goto out').

-- 
Regards,
Christian