X-Authentication-Warning: delorie.com: mail set sender to djgpp-bounces using -f
X-Recipient: djgpp AT delorie DOT com
X-Recipient: dj AT delorie DOT com
X-Original-DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org;
	s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date:
	mime-version; bh=d0AYakCr3/RvSgk50lHHghpEk58QabpPHfIXtZvWXGw=; b=G1HvRdv+Na8o
	XWUc2IU+r3NIbRBe5ExUyeGONCjMroz+CGjT3P3sSvXK/Z9QHYvIXT51ZDrvvM0uoCyVMsZFx6Z3+
	FWF0gqrlMN3KHTN3FKh5sJkQ9mDKnc0+i+Zog/Ui4vLcgIwc2yKhS79falIYAMYLCRGekVPjs+Aiu
	sIkgadrxUCCeEITz4fdK2H+6/6/tzOyGEJGbSO7VdmwdT/JlrwKQeXjnBky2VRS9tm1sAvZsPiehl
	Ai8mDkwTjntN2t60oS8/eSq64MnxG30FoSu+iH4VkZRHj4gHG5dkogX8aJjqvHM0wH0jC58Eh5GrV
	FpfBCJanv+p/ik+ncMsXjw==;
Date: Sat, 13 Apr 2024 15:10:34 +0300
Message-Id: <865xwlpio5.fsf@gnu.org>
From: "Eli Zaretskii (eliz AT gnu DOT org) [via djgpp AT delorie DOT com]" <djgpp AT delorie DOT com>
To: Pali <pali AT pali DOT im>
Cc: dj AT delorie DOT com, sezeroz AT gmail DOT com, djgpp AT delorie DOT com
In-Reply-To: <20240413120109.bbs63syonlprvmw4@pali> (message from Pali on Sat,
	13 Apr 2024 14:01:09 +0200)
Subject: Re: Error handling in __djgpp_set_page_attributes()
References: <20240413103741 DOT wpz7cy3ff3uaflo6 AT pali>
 <86edb9pjka DOT fsf AT gnu DOT org> <20240413120109 DOT bbs63syonlprvmw4 AT pali>
Reply-To: djgpp AT delorie DOT com
Errors-To: nobody AT delorie DOT com
X-Mailing-List: djgpp AT delorie DOT com
X-Unsubscribes-To: listserv AT delorie DOT com
Precedence: bulk

> Date: Sat, 13 Apr 2024 14:01:09 +0200
> From: Pali <pali AT pali DOT im>
> Cc: dj AT delorie DOT com, sezeroz AT gmail DOT com, djgpp AT delorie DOT com
> 
> On Saturday 13 April 2024 14:51:17 Eli Zaretskii wrote:
> > > Date: Sat, 13 Apr 2024 12:37:41 +0200
> > > From: Pali <pali AT pali DOT im>
> > > Cc: djgpp AT delorie DOT com
> > > 
> > > Hello, I have there a change for __djgpp_set_page_attributes() function
> > > which sets more sane errno value on different failures. Could you look
> > > at it if it is useful?
> > 
> > Thanks.  I think this can be useful, but I'd prefer to keep the style
> > of having just one exit point in this function.  So could you please
> > rewrite the patch so that the various conditions only set errno, and
> > still "goto fail"?
> 
> Ok. Something like this?

Yes, but:

> 
> --- src/libc/dpmi/helper/setattr.c
> +++ src/libc/dpmi/helper/setattr.c
> @@ -28,7 +28,7 @@ __djgpp_set_page_attributes (void *_our_addr, unsigned long _num_bytes,
>        || ((_attributes & 0x3) == 2))
>      {
>        errno = EINVAL;
> -      return -1;
> +      goto fail;
>      }
>  
>    /* Set up an array of page attribute information. */
> @@ -50,7 +50,17 @@ __djgpp_set_page_attributes (void *_our_addr, unsigned long _num_bytes,
>        /* Find the memory handle corresponding to the first byte. */
>        d = __djgpp_memory_handle (p);
>        if (d == NULL)
> -	goto fail;
> +        {
> +          errno = EFAULT;
> +          goto fail;
> +        }
> +
> +      /* Base address of the memory handle must be page aligned too. */
> +      if (d->address & 0xfff)
> +        {
> +          errno = EFAULT;
> +          goto fail;
> +        }
>  
>        /* Find the last byte in the range that's also in the same
>         * memory handle as our current starting byte.  We start with
> @@ -68,7 +78,10 @@ __djgpp_set_page_attributes (void *_our_addr, unsigned long _num_bytes,
>  	  /* Find the memory handle corresponding to this test byte. */
>  	  d2 = __djgpp_memory_handle (handle_end_addr);
>  	  if (d2 == NULL)
> -	    goto fail;
> +            {
> +              errno = EFAULT;
> +              goto fail;
> +            }
>  
>  	  /* Is this test byte in the same handle as the first byte? */
>  	  if (d2->handle == d->handle)
> @@ -81,9 +94,35 @@ __djgpp_set_page_attributes (void *_our_addr, unsigned long _num_bytes,
>        meminfo.handle  = d->handle;
>        meminfo.size    = num_pages2;
>        meminfo.address = p - d->address;
> -      if (__dpmi_set_page_attributes (&meminfo, attr)
> -	  || meminfo.size != num_pages2)
> -	goto fail;
> +      if (__dpmi_set_page_attributes (&meminfo, attr))
> +        {
> +          switch (__dpmi_error)
> +            {
> +              case 0x0507: /* Unsupported function (returned by DPMI 0.9 host, error number is same as DPMI function number) */
> +              case 0x8001: /* Unsupported function (returned by DPMI 1.0 host) */
> +                errno = ENOSYS;
> +                break;
> +              case 0x8002: /* Invalid state (page in wrong state for request) */
> +                errno = EFAULT;
> +                break;
> +              case 0x8010: /* Resource unavailable (DPMI host cannot allocate internal resources to complete an operation) */
> +              case 0x8013: /* Physical memory unavailable */
> +              case 0x8014: /* Backing store unavailable */
> +                errno = ENOMEM;
> +                break;
> +              case 0x8021: /* Invalid value (illegal request in bits 0-2 of one or more page attribute words) */
> +                errno = EINVAL;
> +                break;
> +              case 0x8023: /* Invalid handle (in ESI) */
> +              case 0x8025: /* Invalid linear address (specified range is not within specified block) */
> +                errno = EFAULT;
> +                break;
> +              default: /* Other unspecified error */
> +                errno = EIO;
> +                break;

Why EIO and not the original EACCES?

Also, can you collect all EFAULT cases in the switch together?