From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/9384 Path: news.gmane.org!not-for-mail From: "dalias@libc.org" Newsgroups: gmane.linux.lib.musl.general Subject: Re: mips n64 porting review Date: Thu, 25 Feb 2016 13:01:50 -0500 Message-ID: <20160225180150.GT9349@brightrain.aerifal.cx> References: <20160223033233.GM9349@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1456423329 4805 80.91.229.3 (25 Feb 2016 18:02:09 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 25 Feb 2016 18:02:09 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-9397-gllmg-musl=m.gmane.org@lists.openwall.com Thu Feb 25 19:02:07 2016 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1aZ0Ep-00039B-HD for gllmg-musl@m.gmane.org; Thu, 25 Feb 2016 19:02:07 +0100 Original-Received: (qmail 32120 invoked by uid 550); 25 Feb 2016 18:02:04 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 32102 invoked from network); 25 Feb 2016 18:02:03 -0000 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:9384 Archived-At: On Thu, Feb 25, 2016 at 12:05:33PM +0000, Jaydeep Patil wrote: > >> +#include > >> +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