[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160225180150.GT9349@brightrain.aerifal.cx>
Date: Thu, 25 Feb 2016 13:01:50 -0500
From: "dalias@...c.org" <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: mips n64 porting review
On Thu, Feb 25, 2016 at 12:05:33PM +0000, Jaydeep Patil wrote:
> >> +#include <sys/stat.h>
> >> +struct kernel_stat {
> >> + unsigned int st_dev;
> >> + unsigned int __pad1[3];
> >> + unsigned long long st_ino;
> >> + unsigned int st_mode;
> >> + unsigned int st_nlink;
> >> + int st_uid;
> >> + int st_gid;
> >> + unsigned int st_rdev;
> >> + unsigned int __pad2[3];
> >> + long long st_size;
> >> + unsigned int st_atime_sec;
> >> + unsigned int st_atime_nsec;
> >> + unsigned int st_mtime_sec;
> >> + unsigned int st_mtime_nsec;
> >> + unsigned int st_ctime_sec;
> >> + unsigned int st_ctime_nsec;
> >> + unsigned int st_blksize;
> >> + unsigned int __pad3;
> >> + unsigned long long st_blocks;
> >> +};
> >> +
> >> +static void __stat_fix(struct kernel_stat *kst, struct stat *st)
> >> +{
> >> + extern void *memset(void *s, int c, size_t n);
> >> +
> >> + st->st_dev = kst->st_dev;
> >> + memset (&st->st_pad1, 0, sizeof (st->st_pad1));
> >> + st->st_ino = kst->st_ino;
> >> + st->st_mode = kst->st_mode;
> >> + st->st_nlink = kst->st_nlink;
> >> + st->st_uid = kst->st_uid;
> >> + st->st_gid = kst->st_gid;
> >> + st->st_rdev = kst->st_rdev;
> >> + memset (&st->st_pad2, 0, sizeof (st->st_pad2));
> >> + st->st_size = kst->st_size;
> >> + st->st_pad3 = 0;
> >> + st->st_atim.tv_sec = kst->st_atime_sec;
> >> + st->st_atim.tv_nsec = kst->st_atime_nsec;
> >> + st->st_mtim.tv_sec = kst->st_mtime_sec;
> >> + st->st_mtim.tv_nsec = kst->st_mtime_nsec;
> >> + st->st_ctim.tv_sec = kst->st_ctime_sec;
> >> + st->st_ctim.tv_nsec = kst->st_ctime_nsec;
> >> + st->st_blksize = kst->st_blksize;
> >> + st->st_blocks = kst->st_blocks;
> >> + memset (&st->st_pad5, 0, sizeof (st->st_pad5));
> >> + return;
> >> +}
> >
> >You've made this a lot more complicated than it needs to be. Just
> >copying the __stat_fix code from 32-bit mips should work fine. The
> >only fixup that needs to be done is for st_dev and st_rdev and it can
> >be done in-place.
>
> In case of MIPS64, the stat syscall returns a structure as described
> by 'struct kernel_stat'. The ' struct stat' in MIPS64 has following
> notable difference as compared to 'struct kernel_stat':
>
> The st_nlink is of 8 bytes
I missed that in my previous reading.
For all existing archs, nlink_t was the native register size (_Reg) in
the kernel stat struct. It looks like it's just 32-bit (unsigned int)
on mips64 though. The clean way to handle this is just moving nlink_t
back to the per-arch alltypes.h.in files to allow it to vary (or
actually, just putting a definition in the mips one will override the
generic one). So in arch/mips64/bits/alltypes.h.in:
TYPEDEF unsigned nlink_t;
> and all st_*tim members are of 16 bytes
Uhg. Does mips64 actually use a 32-bit time_t? If so I think with
current musl we're stuck as defining time_t as 32-bit, because it's
used as an interface with all sorts of things in the kernel that we
don't wrap/control. But this is going to need a lot more review to
understand if there's any better solution. :(
Even if time_t is 32-bit though, tv_nsec is always long, so the
kernel's stat structure is broken and needs further fixups like you're
doing. I don't see any way around this unless there's hope of the
kernel 64-bit time_t stuff being merged soon, in which case we could
use that from the beginning on mips64. It feels really awful to add a
new 64-bit arch with 32-bit time_t.
In light of this I'm not sure it makes sense to try to match the
kernel's weird nlink_t (see above).
> (due to sizeof long in 64bit). Thus all members of ' struct stat'
> starting from st_uid are at different offset than what stat syscall
> has returned. In case of MIPS32, we just need to adjust dev_t (being
> long long) as described in __stat_fix. However in case of MIPS64 we
> need to members from 'struct kernel_stat' to 'struct stat'
Yes...
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.
