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

Peter Wang novalazy at gmail.com
Wed Apr 30 13:11:15 AEST 2025


On Wed, 30 Apr 2025 12:17:57 +1000 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> 
> 
> 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 don't see how, e.g. on Linux, ENOTEMPTY and EEXIST have different
values, and both would be recognized.

> > @@ -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))".

If $2 is undefined, it won't be expanded. The != expression will be true.

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

I've changed the code to use a secondary switch as you suggested,
as the generated code is a bit simpler. Thanks.

Peter


More information about the reviews mailing list