Re: [FFmpeg-devel] [PATCH v7 2/3] avformat/os_support: Support long file names on Windows

[Soft Works <softworkz@hotmail.com>]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of nil-
> admirari@mailo.com
> Sent: Wednesday, May 25, 2022 4:48 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v7 2/3] avformat/os_support: Support
> long file names on Windows
> 
> > + struct win32_stat
> > + {
> > + _dev_t st_dev; /* ID of device containing file */
> > + _ino_t st_ino; /* inode number */
> > + unsigned short st_mode; /* protection */
> > + short st_nlink; /* number of hard links */
> > + short st_uid; /* user ID of owner */
> > + short st_gid; /* group ID of owner */
> > + _dev_t st_rdev; /* device ID (if special file) */
> > + int64_t st_size; /* total size, in bytes */
> > + int64_t st_atime; /* time of last access */
> > + int64_t st_mtime; /* time of last modification */
> > + int64_t st_ctime; /* time of last status change */
> > + };
> 
> Wouldn't it make sense to add a
> static_assert(sizeof(struct win32_stat) == sizeof(struct _stati64))
> somewhere?

No, it is intended and expected that the structs are different.

> 
> > +static inline int win32_access(const char *filename_utf8, int par)
> > +static inline void copy_stat(struct _stati64 *winstat, struct
> win32_stat *par)
> > +static inline int win32_stat(const char *filename_utf8, struct
> win32_stat *par)
> > +static inline int win32_fstat(int fd, struct win32_stat *par)
> 
> How about renaming par to something more appropriate?

How? And why?

These functions were always named like that (it just wasn't visible
as these were constructed through macros).

> 
> > +static inline void copy_stat(struct _stati64 *winstat, struct
> win32_stat *par)
> > +{
> > + par->st_dev = winstat->st_dev;
> > + par->st_ino = winstat->st_ino;
> > + par->st_mode = winstat->st_mode;
> > + par->st_nlink = winstat->st_nlink;
> > + par->st_uid = winstat->st_uid;
> > + par->st_gid = winstat->st_gid;
> > + par->st_rdev = winstat->st_rdev;
> > + par->st_size = winstat->st_size;
> > + par->st_atime = winstat->st_atime;
> > + par->st_mtime = winstat->st_mtime;
> > + par->st_ctime = winstat->st_ctime;
> > }
> 
> Would memcpy make more sense here?

No, it is intended and expected that the structs are different.

> 
> > +static inline int win32_stat(const char *filename_utf8, struct
> win32_stat *par)
> > +{
> > + struct _stati64 winstat = { 0 };
> > ...
> >
> > +static inline int win32_fstat(int fd, struct win32_stat *par)
> > +{
> > + struct _stati64 winstat = { 0 };
> > ...
> 
> Functions use _stati64 internally, which is affected by _USE_32BIT_TIME_T.
> win32_stat does not take _USE_32BIT_TIME_T into account at all.
> It was already pointed out that _USE_32BIT_TIME_T is actually used:
> https://ffmpeg.org/pipermail/ffmpeg-devel/2022-May/296731.html.

That's why the structs are different and the fields of
win32_stat always large enough, no matter which struct 
is being used internally.

The point of this solution is to provide independence 
between what the application "sees" and what is used 
to call the Windows API.

Kind regards,
softworkz




_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".