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

[nil-admirari at mailo.com]

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

Please document that there is a potential difference in time types
and that the difference is intentional, and that the chosen
time type is always large enough.

Probably it's worthwhile to document that the entire machinery was created
because of POSIX stat function and struct being identically named,
which is not possible to accommodate by a simple macro.

> > > +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).

_access argument is called mode:
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/access-waccess?view=msvc-170.

_stat and _fstat argument is called a buffer:
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/stat-functions?view=msvc-170
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/fstat-fstat32-fstat64-fstati64-fstat32i64-fstat64i32?view=msvc-170
It's no better than par, but you don't have to follow MS.

Somehow winstat refers to parameters of type _stati64, not win32_stat.
I would've called win32_stat params winstat, and _stati64 params crtstat,
but you can always come up with better names.

Inside a macro generic parameter names are a necessity. Functions better be more specific.