[FFmpeg-devel] [FFmpeg-cvslog] lavf/os_support.h: Fix for unicode filenames on windows.

[Clément Bœsch]

On Tue, Jun 14, 2016 at 07:08:58AM +1000, Matt Oliver wrote:
> On 13 June 2016 at 18:29, Benoit Fouet <benoit.fouet at free.fr> wrote:
> 
> > Hi,
> >
> >
> >
> > On 13/06/2016 10:21, Clément Bœsch wrote:
> >
> >> On Mon, Jun 13, 2016 at 05:50:18AM +0200, Matt Oliver wrote:
> >>
> >>> ffmpeg | branch: master | Matt Oliver <protogonoi at gmail.com> | Mon Jun
> >>> 6 17:04:39 2016 +1000| [37787f261639c53998487400e874741c17e85fc6] |
> >>> committer: Matt Oliver
> >>>
> >>> lavf/os_support.h: Fix for unicode filenames on windows.
> >>>
> >>> Fixes #819 #5256 #5281
> >>>
> >>> Signed-off-by: Matt Oliver <protogonoi at gmail.com>
> >>>
> >>>
> >>>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=37787f261639c53998487400e874741c17e85fc6
> >>>>
> >>> ---
> >>>
> >>>   libavformat/file.c       |    4 ++++
> >>>   libavformat/os_support.h |   24 ++++++++++++++++++++++++
> >>>   2 files changed, 28 insertions(+)
> >>>
> >>> diff --git a/libavformat/file.c b/libavformat/file.c
> >>> index 5765ce7..264542a 100644
> >>> --- a/libavformat/file.c
> >>> +++ b/libavformat/file.c
> >>> @@ -148,7 +148,11 @@ static int file_check(URLContext *h, int mask)
> >>>               ret |= AVIO_FLAG_WRITE;
> >>>   #else
> >>>       struct stat st;
> >>> +#   ifndef _WIN32
> >>>       ret = stat(filename, &st);
> >>> +#   else
> >>> +    ret = win32_stat(filename, &st);
> >>> +#   endif
> >>>
> >> why this chunk?
> >>
> >>       if (ret < 0)
> >>>           return AVERROR(errno);
> >>>   diff --git a/libavformat/os_support.h b/libavformat/os_support.h
> >>> index a332911..9e312a5 100644
> >>> --- a/libavformat/os_support.h
> >>> +++ b/libavformat/os_support.h
> >>> @@ -182,6 +182,29 @@ DEF_FS_FUNCTION(unlink, _wunlink, _unlink)
> >>>   DEF_FS_FUNCTION(mkdir,  _wmkdir,  _mkdir)
> >>>   DEF_FS_FUNCTION(rmdir,  _wrmdir , _rmdir)
> >>>   +#define DEF_FS_FUNCTION2(name, wfunc, afunc, partype)     \
> >>> +static inline int win32_##name(const char *filename_utf8, partype par) \
> >>> +{                                                         \
> >>> +    wchar_t *filename_w;                                  \
> >>> +    int ret;                                              \
> >>> +                                                          \
> >>> +    if (utf8towchar(filename_utf8, &filename_w))          \
> >>> +        return -1;                                        \
> >>> +    if (!filename_w)                                      \
> >>> +        goto fallback;                                    \
> >>> +                                                          \
> >>> +    ret = wfunc(filename_w, par);                         \
> >>> +    av_free(filename_w);                                  \
> >>> +    return ret;                                           \
> >>> +                                                          \
> >>> +fallback:                                                 \
> >>> +    /* filename may be be in CP_ACP */                    \
> >>> +    return afunc(filename_utf8, par);                     \
> >>> +}
> >>> +
> >>> +DEF_FS_FUNCTION2(access, _waccess, _access, int)
> >>> +DEF_FS_FUNCTION2(stat, _wstat64, _stat64, struct stat*)
> >>> +
> >>>   static inline int win32_rename(const char *src_utf8, const char
> >>> *dest_utf8)
> >>>   {
> >>>       wchar_t *src_w, *dest_w;
> >>> @@ -231,6 +254,7 @@ fallback:
> >>>   #define rename      win32_rename
> >>>   #define rmdir       win32_rmdir
> >>>   #define unlink      win32_unlink
> >>> +#define access      win32_access
> >>>
> >>>
> >> ...instead of #define stat win32_stat here?
> >>
> >
> > as already noted by someone else, this should be
> > #define stat(a, b) win32_stat((a), (b))
> > in order not to conflict with "struct stat" definition.
> >
> 
> As stated in the original patch thread a define for the win32_stat function
> can not be used as there is already a define for the 'stat' struct.
> So using:
> #define stat(a, b) win32_stat((a), (b))
> clashes with the existing
> #define stat _stat64
> 
> Since there is a stat function and a stat struct then a macro can only be
> used for one of them and there is already an object macro for the stat
> struct.
> If the suggested "#define stat(a, b) win32_stat((a), (b))" was added this
> overrides the struct macro causing the compiler not to use _stat64 and use
> a 32 variant instead which results in memory corruption.

Sorry, I wasn't following the thread, I just happened to read cvslog.

A comment in the source would probably be welcome though, no one is going
to be aware of these discussions in the future.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160613/3675e7a1/attachment.sig>