[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