Re: [PATCH] hurd_types.defs: redefine io_statbuf_t, timespec_t and fsys_statfsbuf_t as true structs

[Samuel Thibault]

Applied, thanks!

Flavio Cruz, le sam. 12 nov. 2022 22:49:02 -0500, a ecrit:
> ---
> 
> Hello Samuel
> 
> Thanks for your comments. I have reworked the patch to use correct unsigned vs
> signed types to be as accurate as possible (was trying to make the size be
> correct only initially). I am now defining new types that are based on what 
> glibc
> provides to make it easier to validate.
> 
> > 
> > Mmm, time_t is actually defined to slongword, i.e. int32_t or int64_t.
> > 
> > It's also very odd to make long an unsigned type.
> > 
> > > +type fsid_t = uint32_t;
> > 
> > ? isn't it 64bit?
> Fixed.
> 
> > 
> > > +type blksize_t = uint32_t;
> > 
> > ? isn't blksize_t a long?
> Fixed.
> 
> > 
> > > +// Dummy structure just to add padding to io_statbuf_t.
> > > +type io_statbuf_spare_int = struct[9] of int;
> > 
> > According to mach/hurd/bits/stat.h, it depends whether it's 32 or 64
> > bits.
> 
> Are you referring to __USE_FILE_OFFSET64? Current structures assume that
> we are using __USE_FILE_OFFSET64. Other than that, it also depends on the 
> size of
> __fsid_t but that is 8 bytes on both 32 or 64 bits, so not the same as
> sizeof (int).
> 
> > 
> > > +// Needs to be kept in sync with bits/stat.h.
> > 
> > Rather explicit glibc/mach/hurd/bits/stat.h, and similarly for statfs.
> Done.
> 
> > 
> > > +type fsblkcnt64_t = uint64_t;
> > > +type fsfilcnt64_t = uint64_t;
> > > +type fsid_t = uint64_t;
> > 
> > This is a redefinition? mig doesn't complain?
> 
> My bad. It worked correctly as I have tested it by compiling hurd but it only
> outputs a warning which I missed. I have a patch that will be submitted later 
> to
> make it fail when it happens.
> 
> > 
> > Samuel
> 
> diff --git a/hurd/hurd_types.defs b/hurd/hurd_types.defs
> index 95399289..fbd217d3 100644
> --- a/hurd/hurd_types.defs
> +++ b/hurd/hurd_types.defs
> @@ -401,7 +401,6 @@ serverprefix SERVERPREFIX;
>  
>  type data_t = array[] of char;
>  type string_t = c_string[1024]; /* XXX */
> -type io_statbuf_t = struct[32] of int;
>  type uid_t = uint32_t;
>  type gid_t = uint32_t;
>  type mode_t = uint32_t;
> @@ -419,12 +418,9 @@ type off_array_t = array[] of loff_t;
>  
>  type pidarray_t = array[] of pid_t;
>  type procinfo_t = array[] of int;
> -type fsys_statfsbuf_t=struct[22] of int;
>  
>  type idarray_t = array[] of uid_t;
>  
> -type rusage_t = struct[18] of int; /* XXX */
> -
>  type flock_t = struct {
>     int l_type;
>     int l_whence;
> @@ -433,7 +429,102 @@ type flock_t = struct {
>     pid_t l_pid;
>  };
>  
> -type timespec_t = struct[2] of int;
> +type unsigned_int = uint32_t;
> +#if defined(__x86_64__)
> +type long = int64_t;
> +type unsigned_long = uint64_t;
> +
> +/* Type names are in sync with glibc/posix/bits/types.h. */
> +type uword = unsigned_long;
> +type uquad = unsigned_long;
> +type squad = long;
> +#else
> +type long = int32_t;
> +type unsigned_long = uint32_t;
> +
> +type uword = unsigned_int;
> +type uquad = uint64_t;
> +type squad = int64_t;
> +#endif /* defined(__x86_64__) */
> +
> +type blksize_t = long;
> +type blkcnt64_t = squad;
> +type dev_t = uint32_t;
> +type fsblkcnt64_t = uquad;
> +type fsfilcnt64_t = uquad;
> +type fsid_t = uquad;
> +type nlink_t = uword;
> +
> +type time_t = long;
> +
> +type timespec_t = struct {
> +  time_t tv_sec;
> +  long tv_nsec;
> +};
> +type timeval = timespec_t;
> +
> +/* Dummy structure just to add padding to io_statbuf_t. */
> +type io_statbuf_spare_int = struct[8] of int;
> +/* Needs to be kept in sync with glibc/mach/hurd/bits/stat.h. */
> +type io_statbuf_t = struct {
> +   int st_fstype;
> +   fsid_t st_fsid;
> +   ino64_t st_ino;
> +   unsigned_int st_gen;
> +   dev_t st_rdev;
> +   mode_t st_mode;
> +   nlink_t st_nlink;
> +   uid_t st_uid;
> +   gid_t st_gid;
> +   loff_t st_size;
> +   timespec_t st_atim;
> +   timespec_t st_mtim;
> +   timespec_t st_ctim;
> +   blksize_t st_blksize;
> +   blkcnt64_t st_blocks;
> +   uid_t st_author;
> +   unsigned_int st_flags;
> +   io_statbuf_spare_int st_spare;
> +};
> +
> +/* Needs to be kept in sync with glibc/mach/hurd/bits/statfs.h. */
> +type fsys_statfsbuf_t = struct {
> +   unsigned_int f_type;
> +   unsigned_long f_bsize;
> +   fsblkcnt64_t f_blocks;
> +   fsblkcnt64_t f_bfree;
> +   fsblkcnt64_t f_bavail;
> +   fsblkcnt64_t f_files;
> +   fsblkcnt64_t f_ffree;
> +   fsid_t f_fsid;
> +   unsigned_long f_namelen;
> +   fsfilcnt64_t f_favail;
> +   unsigned_long f_frsize;
> +   unsigned_long f_flag;
> +   unsigned_int f_spare1;
> +   unsigned_int f_spare2;
> +   unsigned_int f_spare3;
> +};
> +
> +/* Needs to be kept in sync with glibc/resource/bits/types/struct_rusage.h. 
> */
> +type rusage_t = struct {
> +   timeval ru_utime;
> +   timeval ru_stime;
> +   long ru_maxrss;
> +   long ru_ixrss;
> +   long ru_idrss;
> +   long ru_isrss;
> +   long ru_minflt;
> +   long ru_majflt;
> +   long ru_nswap;
> +   long ru_inblock;
> +   long ru_oublock;
> +   long ru_msgsnd;
> +   long ru_msgrcv;
> +   long ru_nsignals;
> +   long ru_nvcsw;
> +   long ru_nivcsw;
> +};
>  
>  #define _SYS_UTSNAME_H               /* Inhibit warning from 
> <bits/utsname.h>.  */
>  #include <bits/utsname.h>
> -- 
> 2.37.2
> 
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.