[m-rev.] for post-commit review: Handle ENOTEMPTY == EEXIST in MR_errno_name().

Zoltan Somogyi zoltan.somogyi at runbox.com
Wed Apr 30 12:17:57 AEST 2025



On Wed, 30 Apr 2025 11:53:27 +1000, Peter Wang <novalazy at gmail.com> wrote:

> diff --git a/tools/generate_errno_name b/tools/generate_errno_name
> index 2337815b2..089deb3d9 100755
> --- a/tools/generate_errno_name
> +++ b/tools/generate_errno_name
> @@ -49,7 +49,6 @@ ERANGE
>  EDEADLK
>  ENAMETOOLONG
>  ENOLCK
> -ENOTEMPTY
>  ELOOP
>  ENOMSG
>  EIDRM

This change means that on systems where ENOTEMPTY exists
and has a value that is NOT shared with other errnos, get_system_error_name
won't recognize it.

I think it would be better if, for each errno that can (i.e. on some systems
*does*) clash with another error, we added an arm in a new switch in the
default case of the main switch, like this:

MR_errno_name(int errnum)
{
  switch (errnum)
  {
    case EEXIST: ...
    ...
    default:
      switch (errnum)
      {
        case ENOTEMPTY: ...
        ....
       }
  }
}

> @@ -163,7 +162,7 @@ EBFONT
>  cat <<EOF
>  // vim: ts=4 sw=4 expandtab ft=c
>  
> -// Copyright (C) 2022 The Mercury team.
> +// Copyright (C) 2022, 2025 The Mercury team.
>  // This file is distributed under the terms specified in COPYING.LIB.
>  
>  // Generated by generate_errno_name.
> @@ -189,14 +188,16 @@ done
>  # - EWOULDBLOCK should be the same as EAGAIN
>  # - EDEADLOCK should be the same as EDEADLK
>  # - ENOTSUP may be the same as EOPNOTSUPP on some platforms
> +# - ENOTEMPTY may be the same as EEXIST on some platforms
>  handle_alias() {
> -    echo "#if defined($1) && !defined($2)"
> +    echo "#if defined($1) && ($1 != $2)"
>      echo "    case $1: return \"$1\";"
>      echo "#endif"
>  }
>  handle_alias EWOULDBLOCK EAGAIN
>  handle_alias EDEADLOCK EDEADLK
>  handle_alias ENOTSUP EOPNOTSUPP
> +handle_alias ENOTEMPTY EEXIST

I have two comments here.

One, the old code here was not just prepared for $2 (the second errno name
in a possibly-aliased pair) to be undefined, but *depended* on it being undefined.
The new code depends on $2 being *defined*. One of these is wrong, though
I don't know which. But whichever it is, it could be fixed by making the second half
of the #if condition "((!defined($2) || $1 != !2))".

Two, this alias handling mechanism handles any potential clash between
EWOULDBLACK/EDEADLOCK/ENOTSUP/ENOTEMPTY with *one* other errno.
The mechanism above, a second switch in the default case, handles a clash
between these four and *any other* errno.

Of course, the extra generality may not be needed, but it may be worthwhile
fixing not just this newly discovered clash, but others that may arise in the future
(for these four errnos, anyway).

Zoltan.





More information about the reviews mailing list