From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/9430 Path: news.gmane.org!not-for-mail From: "Rich Felker (dalias@libc.org)" Newsgroups: gmane.linux.lib.musl.general Subject: Re: MUSL MIPS64 N64 port Date: Tue, 1 Mar 2016 23:41:43 -0500 Message-ID: <20160302044143.GM9349@brightrain.aerifal.cx> References: <20160301023523.GG9349@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 1456893736 32136 80.91.229.3 (2 Mar 2016 04:42:16 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Wed, 2 Mar 2016 04:42:16 +0000 (UTC) Cc: Mahesh Bodapati , "musl@lists.openwall.com" , "nsz@port70.net" To: Jaydeep Patil Original-X-From: musl-return-9443-gllmg-musl=m.gmane.org@lists.openwall.com Wed Mar 02 05:42:06 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 1aaybt-0000rV-Hn for gllmg-musl@m.gmane.org; Wed, 02 Mar 2016 05:42:05 +0100 Original-Received: (qmail 25624 invoked by uid 550); 2 Mar 2016 04:42:02 -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 24573 invoked from network); 2 Mar 2016 04:42:02 -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:9430 Archived-At: On Tue, Mar 01, 2016 at 07:52:33AM +0000, Jaydeep Patil wrote: > Also got: > > src/thread/pthread_create.c:212:11: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] > stack = (void *)(attr._a_stackaddr & -16); > > This is because your pthread types are wrong; you're using the 32-bit arch ones: This does not seem to be fixed; see below in your new patch: > diff --git a/arch/mips64/bits/alltypes.h.in b/arch/mips64/bits/alltypes.h.in > new file mode 100644 > index 0000000..64385c9 > --- /dev/null > +++ b/arch/mips64/bits/alltypes.h.in > [...] > +TYPEDEF struct { union { int __i[9]; volatile int __vi[9]; unsigned long __s[9]; } __u; } pthread_attr_t; > +TYPEDEF struct { union { int __i[6]; volatile int __vi[6]; volatile void *volatile __p[6]; } __u; } pthread_mutex_t; > +TYPEDEF struct { union { int __i[6]; volatile int __vi[6]; volatile void *volatile __p[6]; } __u; } mtx_t; > +TYPEDEF struct { union { int __i[12]; volatile int __vi[12]; void *__p[12]; } __u; } pthread_cond_t; > +TYPEDEF struct { union { int __i[12]; volatile int __vi[12]; void *__p[12]; } __u; } cnd_t; > +TYPEDEF struct { union { int __i[8]; volatile int __vi[8]; void *__p[8]; } __u; } pthread_rwlock_t; > +TYPEDEF struct { union { int __i[5]; volatile int __vi[5]; void *__p[5]; } __u; } pthread_barrier_t; These are still the 32-bit definitions not the 64-bit ones. > diff --git a/arch/mips64/bits/stat.h b/arch/mips64/bits/stat.h > new file mode 100644 > index 0000000..dfc80b3 > --- /dev/null > +++ b/arch/mips64/bits/stat.h > @@ -0,0 +1,23 @@ > +#include > +#include > + > +struct stat { > + dev_t st_dev; > + int pad1[3]; > + ino_t st_ino; > + mode_t st_mode; > + nlink_t st_nlink; > + uid_t st_uid; > + gid_t st_gid; > + dev_t st_rdev; > + unsigned int pad2[2]; > + off_t st_size; > + int pad3; > + struct timespec st_atim; > + struct timespec st_mtim; > + struct timespec st_ctim; > + blksize_t st_blksize; > + unsigned int pad4; > + blkcnt_t st_blocks; > + int pad5[14]; > +}; pad1, pad2 etc. are in the application-reserved namespace. These need to be __pad1, __pad2, etc. or similar. -- something reserved. > +static void __stat_fix(struct kernel_stat *kst, struct stat *st) > +{ > + st->st_dev = kst->st_dev; > + st->pad1[0] = 0; > + st->pad1[1] = 0; > + st->pad1[2] = 0; > + 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; > + st->pad2[0] = 0; > + st->pad2[1] = 0; > + st->st_size = kst->st_size; > + 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; > + return; > +} When I said the memset can be removed I didn't mean to replace it with assignments. Although they're not as bad, I don't see any reason they're needed at all. The padding does not need to contain meaningful content. Just leaving it uninitialized is fine. Also, the return statement is not needed. > +static inline long __syscall2(long n, long a, long b) > +{ > + struct kernel_stat kst = {0,}; I didn't catch this before, but is there a reason you're filling kst with zeros before passing it to the kernel? I don't think it's useful, and it generates extra code the compiler cannot optimize out (because it cannot know the kernel won't read the memory). Same applies for all the oher __syscallN's. The rest of your changes all look okay. Since it looks like it's about ready to commit, I have a couple general questions about merging. How should the port be credited? To imgtec.com? Should someone on your team be the commit author in the commit, or should I just commit it in my name with your team credited in the commit message and COPYRIGHT file? Thanks for your work on this and your patience with review. :) Rich