Message ID | acd81c61c39eaf06490aae1ab845c32b65b4398a.1652435595.git.ffmpegagent@gmail.com |
---|---|
State | New |
Headers | show |
Series | Support long file names on Windows | expand |
Context | Check | Description |
---|---|---|
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
> 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?
> -----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
> 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 --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; }