Re: [PATCH 08/33] Add structs target_freebsd11_nstat and target_freebsd11_statfs to bsd-user/syscall_defs.h

[Warner Losh <imp@bsdimp.com>]

[-- Attachment #1: Type: text/plain, Size: 2163 bytes --]

On Tue, Aug 8, 2023 at 9:12 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 8/8/23 19:51, Warner Losh wrote:
> >      > +    /* __int32_t  st_lspare; */
> >
> >     Why commented out?
> >
> >
> > I believe that the element was a padding one ....
> >
> >      > +    struct target_freebsd_timespec st_birthtim; /* time of file
> creation */
> >
> >     Does that not place st_birthtim at the wrong place?
> >
> >
> > So this winds up in the right place because there's a hole...
> >
> > However, having said that, I don't think it should be commented out.
> It's not
> > in the bsd-user branch. And the state of the upstream code is such that
> we can't
> > run full tests easily on the system calls, so we're making sure they
> basically
> > work, but will run the full regression test once some other changes are
> made
> > to allow shared libraries to work (many of the calls in this patch are
> needed
> > to make 'hello world' work).
>
> I think there is not a hole, because the struct is __packed.
>
> (Also, QEMU_PACKED vs __packed?)


There's a nstat that's an older stat w/o this field. I'm not entirely sure
__packed should
be on either one of these, honestly. nstat is quite old, and I'm not at all
sure what's up
with it. I think it dates from a time when there was only i386 and then we
expanded to
alpha and needed to 'fix' this interface... Not sure why it got the
freebsd11_ prefix, so
I'll have to chase those details down to see if this is an extra cut and
paste or what.
That may take a little bit to chase down in the logs and in people's
memory. I think
that's the back story. Normally, nstat is only defined in the kernel, and
this is a binary
interface from ages ago that we likely don't need to implement, but I need
to confirm
that, and make sure rust or go don't have some weird, misguided mistake...

But nstat and stat are supposed to be the same, except nstat omits
st_spare, so that's
why it's commented out. stat's supposed to be carefully laid out so packed
or not
doesn't matter. But testing that would take a little bit as well.

Warner


> ~
>

[-- Attachment #2: Type: text/html, Size: 2983 bytes --]