Re: [FFmpeg-devel] [PATCH v2 1/2] avutil/wchar_filename, file_open: 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: Tuesday, May 17, 2022 5:07 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] avutil/wchar_filename,
> file_open: Support long file names on Windows
> 
> > stat wasn't already defined as win32_stat.
> > win32_stat was already defined but not mapped. That's what my change
> > does.
> 
> There are two defines in os_support.h:
> 
> #  ifdef stat
> #   undef stat
> #  endif
> #  define stat _stati64
> 
> and
> 
> DEF_FS_FUNCTION2(stat, _wstati64, _stati64, struct stat*)
> 
> which defines win32_stat (not stat). This function takes struct stat*,
> which due to previous define
> expands into struct _stati64*.
> 
> _stati64 and _wstati64 both take struct _stati64*, which is named
> identically to the first function.
> struct _stati64 expands into different structs depending on the value
> of _USE_32BIT_TIME_T,
> which your explicit structure definition does not capture, see:
> https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/stat-
> functions?view=msvc-170.
> If someone defines_USE_32BIT_TIME_T, your code will fail to compile.

Yes, that's true. But there are hundreds of other things someone could
define which makes compilation fail. 
We don't need to accommodate for every single possibility, and it's not
that _USE_32BIT_TIME_T would be required or the default for 32bit 
compilation.


> C allows functions and structs to have identical names, preprocessor
> does not;
> therefore win32_stat must be used explicitly where stat is required as
> in file.c:160

Except when you define a compatible struct with the same name as the
function - like I did.


>     struct stat st; // expands into struct _stati64 on Windows.
> #   ifndef _WIN32
>     ret = stat(filename, &st);
> #   else
>     ret = win32_stat(filename, &st);
> #   endif

This could be removed after the patch.


> However, no everyone follows: img2dec.c:504 and ipfsgateway.c:104 use
> plain stat.
> 
> if (stat(filename, &img_stat)) {
> stat_ret = stat(ipfs_full_data_folder, &st);
> 
> In these files, on Windows, both the struct and the function call
> expand into _stati64,
> and this time the function call bypasses the UTF-8 to wchar
> conversion.
> 
> Apparently yet another macro is necessary:
> 
> #ifdef _WIN32
> #define ff_stat win32_stat
> #else
> #define ff_stat stat
> #endif

Probably you didn't spot it. It's already there:

#  define stat win32_stat

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".