diff mbox series

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

Message ID acd81c61c39eaf06490aae1ab845c32b65b4398a.1652435595.git.ffmpegagent@gmail.com
State New
Headers show
Series Support long file names on Windows | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Aman Karmani May 13, 2022, 9:53 a.m. UTC 
From: softworkz <softworkz@hotmail.com>

Signed-off-by: softworkz <softworkz@hotmail.com>
---
 libavformat/os_support.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Nil Admirari May 15, 2022, 7:13 p.m. UTC | #1 
> diff --git a/libavformat/os_support.h b/libavformat/os_support.h

In addition to what you've already added, this file defines stat as:

#ifdef _WIN32
...
#  ifdef stat
#   undef stat
#  endif
#  define stat _stati64
...

which is
1. not wide-char aware (_wstati64 does exist)
2. not long path aware.

Also there is a function:

static inline int is_dos_path(const char *path)
{
#if HAVE_DOS_PATHS
    if (path[0] && path[1] == ':')
        return 1;
#endif
    return 0;
}

Now, DOS paths C:... end up being \\?\C:.... Are you sure it won't break something down the line?
Soft Works May 15, 2022, 10:14 p.m. UTC | #2 
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of nil-
> admirari@mailo.com
> Sent: Sunday, May 15, 2022 9:13 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 2/2] avformat/os_support: Support
> long file names on Windows
> 
> > diff --git a/libavformat/os_support.h b/libavformat/os_support.h
> 
> In addition to what you've already added, this file defines stat as:
> 
> #ifdef _WIN32
> ...
> #  ifdef stat
> #   undef stat
> #  endif
> #  define stat _stati64
> ...
> 
> which is
> 1. not wide-char aware (_wstati64 does exist)
> 2. not long path aware.

That's a good point, even though plain stat is used in 2 cases only.
We already have win32_stat, but what's a bit tricky is that the 
struct that this function takes as a parameter is named the same 
as the function itself.

Now I have repeated the definition of 'struct stat' as 
'struct win32_stat', but maybe someone can come up with a better way.
Would you have any idea perhaps?


> Also there is a function:
> 
> static inline int is_dos_path(const char *path)
> {
> #if HAVE_DOS_PATHS
>     if (path[0] && path[1] == ':')
>         return 1;
> #endif
>     return 0;
> }
> 
> Now, DOS paths C:... end up being \\?\C:.... Are you sure it won't
> break something down the line?

The prefixing of paths is always done right before calling any of
the Windows APIs. Original strings are never modified like this.
You can be sure, because all those operations are done on
WCHAR strings only.

You could ask the question only about the case where a user might 
supply a path that is prefixed already, but when we look at the 
only(!) usage of this function then it is about handling URL and 
checking URL paths and a path starting with \\?\ wouldn't be
valid in a URL anyway.
So I don't think that this is of any concern, but it was a good
point to verify this.

Thanks for the review!

softworkz
Nil Admirari May 16, 2022, 8:19 a.m. UTC | #3 
> We already have win32_stat, but what's a bit tricky is that the 
> struct that this function takes as a parameter is named the same 
> as the function itself.

Sorry, I thought is was a definition of a function, not a struct.
Since stat function is already defined as win32_stat,
It's better to revert the changes.
diff mbox series

Patch

diff --git a/libavformat/os_support.h b/libavformat/os_support.h
index 5e6b32d2dc..6c1e6c3851 100644
--- a/libavformat/os_support.h
+++ b/libavformat/os_support.h
@@ -153,7 +153,7 @@  static inline int win32_##name(const char *filename_utf8) \
     wchar_t *filename_w;                                  \
     int ret;                                              \
                                                           \
-    if (utf8towchar(filename_utf8, &filename_w))          \
+    if (get_extended_win32_path(filename_utf8, &filename_w)) \
         return -1;                                        \
     if (!filename_w)                                      \
         goto fallback;                                    \
@@ -177,7 +177,7 @@  static inline int win32_##name(const char *filename_utf8, partype par) \
     wchar_t *filename_w;                                  \
     int ret;                                              \
                                                           \
-    if (utf8towchar(filename_utf8, &filename_w))          \
+    if (get_extended_win32_path(filename_utf8, &filename_w)) \
         return -1;                                        \
     if (!filename_w)                                      \
         goto fallback;                                    \
@@ -199,9 +199,9 @@  static inline int win32_rename(const char *src_utf8, const char *dest_utf8)
     wchar_t *src_w, *dest_w;
     int ret;
 
-    if (utf8towchar(src_utf8, &src_w))
+    if (get_extended_win32_path(src_utf8, &src_w))
         return -1;
-    if (utf8towchar(dest_utf8, &dest_w)) {
+    if (get_extended_win32_path(dest_utf8, &dest_w)) {
         av_free(src_w);
         return -1;
     }