Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130304220610.GQ20323@brightrain.aerifal.cx>
Date: Mon, 4 Mar 2013 17:06:10 -0500
From: Rich Felker <dalias@...ifal.cx>
To: musl@...ts.openwall.com
Subject: Re: Fix for fields in utmp

On Sun, Mar 03, 2013 at 12:29:57AM +0100, Szabolcs Nagy wrote:
> > > > -int_fast16_t 8
> > > > -int_fast32_t 8
> > > > +int_fast16_t 4
> > > > +int_fast32_t 4
> > > 
> > > these diffs may cause problems
> > 
> > These are actually omitted from the ABI documents if I remember
> 
> ok then keep it as is

This still should be checked. It _could_ be changed if it matters..

> > > > -regex_t 64
> > > > +regex_t 56
> > 
> > I think this should be fixed to match.
> 
> glibc's definition is something like:
> 
> typedef struct {
> 	char *__x1;
> 	long __x2;
> 	long __x3;
> 	long __x4;
> 	char *__x5;
> 	char *__x6;
> 	size_t re_nsub;
> 	char __x7;
> } regex_t;
> 
> musl's definition (only re_nsub and __opaque are used):
> 
> typedef struct {
> 	size_t re_nsub;
> 	void *__opaque, *__padding[4];
> 	size_t __nsub2;
> } regex_t;

Fixed, even though it doesn't matter with the other ABI
incompatibility issues.

> > > > -regmatch_t 8
> > > > -regoff_t 4
> > > > +regmatch_t 16
> > > > +regoff_t 8
> > 
> > These are a bug in glibc; their types do not conform to the
> > requirements imposed on regex.h and regexec().
> 
> so regex abi wont be compatible

Yes, this will need some special dynamic linker glue if we want to be
able to run glibc libs/bins using regex. But it's likely they might
not work anyway without glibc extensions...

> > > > -siginfo_t 128
> > > > +siginfo_t 136
> > 
> > This is probably a mistake, maybe in the padding..?
> 
> yes, the si_fields union is 8byte aligned

Fixed.

> > > > -struct arpd_request 40
> > > > +struct arpd_request 28
> > 
> > No idea.
> 
> i think this is not used anymore

Fixed anyway.

> > > > -struct lastlog 292
> > > > +struct lastlog 296
> > 
> > Probably needs to be checked.
> 
> glibc's definition:
> (using the same hack for time_t as in utmpx)

This can probably be ignored. The files will be incompatible, but it's
intended that they not be used anyway..

> > > > -struct ntptimeval 72
> > > > +struct ntptimeval 32
> > 
> > Should we change this? Probably.
> 
> musl does not have ntp_gettime so it probably
> should not define this struct

You're probably right, but I'm holding off a bit on removing it in
case others have input..

> > > > -struct rusage 144
> > > > -struct sched_param 4
> > > > +struct rusage 272
> > > > +struct sched_param 48
> > 
> > Not sure about rusage. For sched_param, we support the extra fields
> > for the sporadic server option even though we don't support it (and
> > Linux probably never will).
> 
> musl's rusage (the difference is the __reserved part):

Should we change it or keep the extra reserved space?

> > > > -struct sockaddr_storage 128
> > > > +struct sockaddr_storage 136
> > 
> > Probably wrong.
> 
> wrong padding

Fixed.

> > > > -wctrans_t 8
> > > > -wctype_t 8
> > > > +wctrans_t 4
> > > > +wctype_t 4
> > > 
> > > it seems wctype_t is unconditionally long in glibc
> > 
> > As it should be. This is a bug in musl. Being an opaque type that
> > could (and often should) be implemented as a pointer, wctype_t should
> > always be pointer-sized.
> 
> then this should be fixed

I will fix this, but as a separate commit since it actually has
nontrivial effects.

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.