Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170218043212.GQ1520@brightrain.aerifal.cx>
Date: Fri, 17 Feb 2017 23:32:12 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: syscall table discrepancies

On Sat, Feb 18, 2017 at 03:41:29AM +0100, Szabolcs Nagy wrote:
> * Rich Felker <dalias@...c.org> [2017-02-16 21:00:44 -0500]:
> > On Thu, Feb 16, 2017 at 08:51:47PM -0500, Rich Felker wrote:
> > > arm
> > > +o__NR_arm_fadvise64_64 270
> > > +o__NR_arm_sync_file_range 341
> > > -o__NR_fadvise64_64 270
> > 
> > Also a naming matter and one I'd probably rather not change, though
> > I'm not sure. It looks like powerpc has the same nonstandard arg order
> > for fadvise but doesn't use a different name, so it's not really
> > helpful for arm to use a different name here.
> > 
> > Not sure about sync_file_range; it might suggest we have a bug.
> 
> it's just an alias, i added the arm names, kept the normal name

OK. Alternatively, if we wanted to have the public header more like
Linux, src/internal/syscall.h could just have

#ifdef SYS_arm_fadvise64_64
#define SYS_fadvise64_64 SYS_arm_fadvise64_64
#endif

Not sure if this is better or worse.

> > > i386
> > > -o__NR_madvise1 219
> > 
> > I think this is cruft that was removed...?
> 
> linux removed it, i removed it too

Fine.

> > > or1k
> > > -o__NR__llseek 62
> > > +o__NR_llseek 62
> > 
> > This looks like a bug that probably has lseek broken on or1k with
> > files larger than 2GB... I think the #else case in lseek.c should
> 
> it's a naming issue, i added the new name, but kept the old one
> for now.

Indeed, I misread the direction of the diff. But perhaps we should use
the upstream/reasonable name and have:

#ifdef SYS__llseek
#define SYS_llseek SYS__llseek
#endif

in src/internal/syscall.h, and use SYS_llseek in the source files.
Thoughts?

> __NR_lseek would be a bug

Yes, I thought we were missing the define for SYS__llseek and thus
that lseek.c would fallback to this and have a bug.

> __NR__llseek is the old name of the syscall.
> __NR_llseek is how new 32bit arches call it.

*nod* makes sense.

> > probably be tweaked to produce a compile-time error if syscall_arg_t
> > is 32-bit. That would also catch musl's equivalent of this n32 bug
> > which remains unfixed:
> > 
> > https://sourceware.org/ml/libc-alpha/2017-01/msg00074.html
> > 
> > > powerpc
> > > -o__NR_getresgid32 170
> > > -o__NR_getresuid32 165
> > > -o__NR_setresgid32 169
> > > -o__NR_setresuid32 164
> > 
> > These can probably be safely removed; I suspect they were cruft from
> > before the general renaming in src/internal/syscall.h was in place.
> > 
> > > -o__NR_timerfd 306
> > 
> > Also a leftover/legacy name, I presume?
> 
> i dropped these.

Looks good. I don't think there's anything wrong with the patch as-is.
If no one else sees any problems I think I'll commit it as-is and we
can discuss further improvements separately; holding up the patch for
tweaks would be pretty bleh.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.