Message ID | 20230808060815.9001-9-kariem.taha2.7@gmail.com |
---|---|
State | New |
Headers | show |
Series | Implement the stat system calls for FreeBSD. | expand |
On 8/7/23 23:07, Karim Taha wrote: > + uint32_t st_flags; /* user defined flags for file */ > + __uint32_t st_gen; /* file generation number */ Drop the __. > + /* __int32_t st_lspare; */ Why commented out? > + struct target_freebsd_timespec st_birthtim; /* time of file creation */ Does that not place st_birthtim at the wrong place? r~
On Tue, Aug 8, 2023 at 3:24 PM Richard Henderson < richard.henderson@linaro.org> wrote: > On 8/7/23 23:07, Karim Taha wrote: > > + uint32_t st_flags; /* user defined flags for file */ > > + __uint32_t st_gen; /* file generation number */ > > Drop the __. > > > + /* __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). Warner > > r~ >
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?) r~
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 > ~ >
On Tue, Aug 8, 2023 at 10:41 PM Warner Losh <imp@bsdimp.com> wrote: > > > 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. > OK. Back in 1998, John Dyson added nstat and friends: commit 1f5621728039a2009fc163d345508d0ee9fae2e9 Author: John Dyson <dyson@FreeBSD.org> Date: Mon May 11 03:55:28 1998 +0000 Fix the futimes/undelete/utrace conflict with other BSD's. Note that the only common usage of utrace (the possible problem with this commit) is with malloc, so this should be a real problem. Add the various NetBSD syscalls that allow full emulation of their development environment. such emulation hasn't worked since the ELF cut over in FreeSBD 3.2 in 1999,if it even did before that (there's some code to implement these, but it's not at all clear to me it ever worked)... This was all preserved when FreeBSD 11 converted to 64-bit inodes. So I guess we should preserve it, but I still need to find out the layout of nstat vs stat on different platforms (though the only systems we ran on at the time were i386, so after grubbing around with git blame, I completely retract my 'it was related to alpha' comments: they were completely wrong). But even so, I guess it was only ever used in life fire with a.out to run old netbsd a.out files from 25 years ago and it's relevance is zero since nobody has used it since then and all maintenance since then in FreeBSD has been basically useless. I may try to do a regression build of rust w/o it to see if it's used there or not... So today I learned... Warner
diff --git a/bsd-user/syscall_defs.h b/bsd-user/syscall_defs.h index e796e7e13d..06be8244de 100644 --- a/bsd-user/syscall_defs.h +++ b/bsd-user/syscall_defs.h @@ -250,6 +250,71 @@ struct target_stat { uint64_t st_spare[10]; }; + +/* struct nstat is the same as stat above but without the st_lspare field */ +struct target_freebsd11_nstat { + uint32_t st_dev; /* inode's device */ + uint32_t st_ino; /* inode's number */ + int16_t st_mode; /* inode protection mode */ + int16_t st_nlink; /* number of hard links */ + uint32_t st_uid; /* user ID of the file's owner */ + uint32_t st_gid; /* group ID of the file's group */ + uint32_t st_rdev; /* device type */ + struct target_freebsd_timespec st_atim; /* time last accessed */ + struct target_freebsd_timespec st_mtim; /* time last data modification */ + struct target_freebsd_timespec st_ctim; /* time last file status change */ + int64_t st_size; /* file size, in bytes */ + int64_t st_blocks; /* blocks allocated for file */ + uint32_t st_blksize; /* optimal blocksize for I/O */ + uint32_t st_flags; /* user defined flags for file */ + __uint32_t st_gen; /* file generation number */ + /* __int32_t st_lspare; */ + struct target_freebsd_timespec st_birthtim; /* time of file creation */ + /* + * Explicitly pad st_birthtim to 16 bytes so that the size of + * struct stat is backwards compatible. We use bitfields instead + * of an array of chars so that this doesn't require a C99 compiler + * to compile if the size of the padding is 0. We use 2 bitfields + * to cover up to 64 bits on 32-bit machines. We assume that + * CHAR_BIT is 8... + */ + unsigned int:(8 / 2) * (16 - (int)sizeof(struct target_freebsd_timespec)); + unsigned int:(8 / 2) * (16 - (int)sizeof(struct target_freebsd_timespec)); +} __packed; + +/* + * sys/mount.h + */ + +/* filesystem id type */ +typedef struct target_freebsd_fsid { int32_t val[2]; } target_freebsd_fsid_t; + +/* filesystem statistics */ +struct target_freebsd11_statfs { + uint32_t f_version; /* structure version number */ + uint32_t f_type; /* type of filesystem */ + uint64_t f_flags; /* copy of mount exported flags */ + uint64_t f_bsize; /* filesystem fragment size */ + uint64_t f_iosize; /* optimal transfer block size */ + uint64_t f_blocks; /* total data blocks in filesystem */ + uint64_t f_bfree; /* free blocks in filesystem */ + int64_t f_bavail; /* free blocks avail to non-superuser */ + uint64_t f_files; /* total file nodes in filesystem */ + int64_t f_ffree; /* free nodes avail to non-superuser */ + uint64_t f_syncwrites; /* count of sync writes since mount */ + uint64_t f_asyncwrites; /* count of async writes since mount */ + uint64_t f_syncreads; /* count of sync reads since mount */ + uint64_t f_asyncreads; /* count of async reads since mount */ + uint64_t f_spare[10]; /* unused spare */ + uint32_t f_namemax; /* maximum filename length */ + uint32_t f_owner; /* user that mounted the filesystem */ + target_freebsd_fsid_t f_fsid; /* filesystem id */ + char f_charspare[80]; /* spare string space */ + char f_fstypename[16]; /* filesys type name */ + char f_mntfromname[88]; /* mount filesystem */ + char f_mntonname[88]; /* dir on which mounted*/ +}; + #define safe_syscall0(type, name) \ type safe_##name(void) \ { \