[PATCH v2] Cygwin: mmap: fix mmap_is_attached_or_noreserve

Corinna Vinschen corinna-cygwin@cygwin.com
Tue Jan 7 19:15:36 GMT 2025


Hi Ken,

Happy New Year!

On Dec 27 11:46, Ken Brown wrote:
> I have a question about the "Fixes" line.  Since the commit in question was
> in the old CVS style, it doesn't have a good one-line summary.  I tried to
> choose the next-best thing, but I'm not sure about it.

Yeah, just a nice approximation of the first line in the old CVS
commit is sufficient.

Your patch looks good, only...

> @@ -784,23 +788,27 @@ mmap_is_attached_or_noreserve (void *addr, size_t len)
>  	  ret = MMAP_RAISE_SIGBUS;
>  	  break;
>  	}
> -      if (!rec->noreserve ())
> -	break;
> +      if (nocover)
> +	/* We need to continue in case we encounter an attached mmap
> +	   later in the list. */
> +	continue;
>  
> -      size_t commit_len = u_len - (start_addr - u_addr);
> -      if (commit_len > len)
> -	commit_len = len;
> +      if (!rec->noreserve ())
> +	{
> +	  nocover = true;
> +	  continue;
> +	}

What about merging the two conditionals into one?  E. g.

  if (nocover || !rec->noreserve ())
    {
      nocover = true;
      continue;
    }

It's a minor style issue, if you like your version better, go for it.


Thanks,
Corinna


More information about the Cygwin-patches mailing list