Error handling in __djgpp_map_physical_memory()

[Eli Zaretskii ([e-mail]) [via [e-mail]]]

Date: Sun, 21 Apr 2024 12:24:44 +0200
Hello, I have there a change for __djgpp_map_physical_memory function,
it is improving setting errno in the same way as was done already for
the function __djgpp_set_page_attributes.

Thanks, I have a couple of minor comments below.

+ /* Base address of the memory handle must be page aligned too. */
+ if (d->address & 0xfff)
+ {
+ errno = EFAULT;

I think this should be EINVAL. An unaligned address is not really
"bad address", it's just that it's not valid for this function.

+ case 0x8023: /* Invalid handle (in ESI) */
+ case 0x8025: /* Invalid linear address (specified range is not within specified block or EBX/EDX is not page-aligned) */
+ errno = EFAULT;

These two should also be EINVAL, IMO.

[Eli Zaretskii ([e-mail]) [via [e-mail]]]

Date: Sun, 21 Apr 2024 21:33:38 +0200

Post by Eli Zaretskii (***@gnu.org) [via ***@delorie.com]

+ /* Base address of the memory handle must be page aligned too. */
+ if (d->address & 0xfff)
+ {
+ errno = EFAULT;

I think this should be EINVAL. An unaligned address is not really
"bad address", it's just that it's not valid for this function.

This is base address of the memory handle found by the DJGPP's
__djgpp_memory_handle function. Not the address passed as input
parameter by caller. So I thought that EFAULT is better there.

Post by Eli Zaretskii (***@gnu.org) [via ***@delorie.com]

+ case 0x8023: /* Invalid handle (in ESI) */
+ case 0x8025: /* Invalid linear address (specified range is not within specified block or EBX/EDX is not page-aligned) */
+ errno = EFAULT;

These two should also be EINVAL, IMO.

EFAULT errno is mostly returned when the passed pointer is invalid, e.g.
out of the process space. So I thought that EFAULT is better error code
than EINVAL for the case when the DPMI server say that it rejected
linear address. Linear address for this DPMI call is defined by the
handle and offset.
That is the reason why I chose these errno codes.

Maybe we should first agree what is "bad address", then? To me, it
means address that is outside of the program's address space, or
something that cannot be an address at all. That's not what happens
here.

Anyway, I used exactly same errno codes as in merged change for the
__djgpp_set_page_attributes function. And I think that these two
functions should use same errno codes for common errors. So in case you
think that there should be EINVAL (and not EFAULT) then it should be
changed for both functions.

If agree about the above, then yes, they both should be modified.

[Eli Zaretskii ([e-mail]) [via [e-mail]]]

Date: Fri, 3 May 2024 11:44:43 +0200

Post by Eli Zaretskii (***@gnu.org) [via ***@delorie.com]
If agree about the above, then yes, they both should be modified.

Could you please do it? Also updates documentation setattr.txh file
where is newly mentioned EFAULT code.

Hello Eli, od you need from me to send a new change with replacing
EFAULT by EINVAL? Or would you do this replacement during applying
current change? I think that this is the only issue with change.

I can do it, but I need to find free time for that (of which there is
not too much lately, given my other engagements). Or you could send
an updated patch. Your call.

Thanks.

[Eli Zaretskii ([e-mail]) [via [e-mail]]]

Date: Sat, 4 May 2024 11:39:12 +0200

Post by Eli Zaretskii (***@gnu.org) [via ***@delorie.com]
I can do it, but I need to find free time for that (of which there is
not too much lately, given my other engagements). Or you could send
an updated patch. Your call.
Thanks.

No problem. I prepared the updated version with just replacing all

Thanks, installed.