From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 6FE06431D5 for ; Tue, 24 May 2022 20:58:59 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id EC21168B554; Tue, 24 May 2022 23:58:57 +0300 (EEST) Received: from mail8.parnet.fi (mail8.parnet.fi [77.234.108.134]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 4960568B443 for ; Tue, 24 May 2022 23:58:51 +0300 (EEST) Received: from mail9.parnet.fi (mail9.parnet.fi [77.234.108.21]) by mail8.parnet.fi with ESMTP id 24OKwouA004656-24OKwouB004656; Tue, 24 May 2022 23:58:50 +0300 Received: from foo.martin.st (host-97-187.parnet.fi [77.234.97.187]) by mail9.parnet.fi (Postfix) with ESMTPS id AB649A142D; Tue, 24 May 2022 23:58:50 +0300 (EEST) Date: Tue, 24 May 2022 23:58:50 +0300 (EEST) From: =?ISO-8859-15?Q?Martin_Storsj=F6?= To: softworkz In-Reply-To: <6f8d400db74be1d704b4c98e2f1295803829ce75.1653400709.git.ffmpegagent@gmail.com> Message-ID: <9fe1e03b-ebd4-ceda-b62b-a34d5012b7be@martin.st> References: <6f8d400db74be1d704b4c98e2f1295803829ce75.1653400709.git.ffmpegagent@gmail.com> MIME-Version: 1.0 X-FE-Policy-ID: 3:14:2:SYSTEM Subject: Re: [FFmpeg-devel] [PATCH v6 2/2] avformat/os_support: Support long file names on Windows X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Soft Works , Hendrik Leppkes , ffmpeg-devel@ffmpeg.org Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Tue, 24 May 2022, softworkz wrote: > From: softworkz > > Signed-off-by: softworkz > --- > libavformat/os_support.h | 87 +++++++++++++++++++++++++++++----------- > 1 file changed, 63 insertions(+), 24 deletions(-) > > diff --git a/libavformat/os_support.h b/libavformat/os_support.h > index 5e6b32d2dc..179b926293 100644 > --- a/libavformat/os_support.h > +++ b/libavformat/os_support.h > @@ -49,7 +49,24 @@ > # ifdef stat > # undef stat > # endif > -# define stat _stati64 > + > +# define stat win32_stat > + > + struct win32_stat > + { > + _dev_t st_dev; /* ID of device containing file */ > + _ino_t st_ino; /* inode number */ > + unsigned short st_mode; /* protection */ > + short st_nlink; /* number of hard links */ > + short st_uid; /* user ID of owner */ > + short st_gid; /* group ID of owner */ > + _dev_t st_rdev; /* device ID (if special file) */ > + long st_size; /* total size, in bytes */ > + time_t st_atime; /* time of last access */ > + time_t st_mtime; /* time of last modification */ > + time_t st_ctime; /* time of last status change */ > + }; Please use int64_t for both st_size and st_?time. We already use _stati64 so far, so we get 64 bit sizes (and long definitely isn't a 64 bit type on Windows!), and with int64_t in the outer struct, we won't accidentally truncate any valid data that we got from the lower level stat function call. > + > # ifdef fstat > # undef fstat > # endif > @@ -153,7 +170,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; \ > @@ -171,37 +188,59 @@ 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); \ > +static inline int win32_access(const char *filename_utf8, int par) > +{ > + wchar_t *filename_w; > + int ret; > + if (get_extended_win32_path(filename_utf8, &filename_w)) > + return -1; > + if (!filename_w) > + goto fallback; > + ret = _waccess(filename_w, par); > + av_free(filename_w); > + return ret; > +fallback: > + return _access(filename_utf8, par); > } > > -DEF_FS_FUNCTION2(access, _waccess, _access, int) > -DEF_FS_FUNCTION2(stat, _wstati64, _stati64, struct stat*) > +static inline int win32_stat(const char *filename_utf8, struct stat *par) > +{ Maybe "struct win32_stat" in the parameter here too, for consistency? > + wchar_t *filename_w; > + int ret; > + struct _stati64 winstat = { 0 }; > + > + if (get_extended_win32_path(filename_utf8, &filename_w)) > + return -1; > + > + if (filename_w) { > + ret = _wstat64(filename_w, &winstat); > + av_free(filename_w); > + } else > + ret = _stat64(filename_utf8, &winstat); > + > + par->st_dev = winstat.st_dev; > + par->st_ino = winstat.st_ino; > + par->st_mode = winstat.st_mode; > + par->st_nlink = winstat.st_nlink; > + par->st_uid = winstat.st_uid; > + par->st_gid = winstat.st_gid; > + par->st_rdev = winstat.st_rdev; > + par->st_size = winstat.st_size; > + par->st_atime = winstat.st_atime; > + par->st_mtime = winstat.st_mtime; > + par->st_ctime = winstat.st_ctime; Thanks, this approach seems robust and safe to me! With this change in place, shouldn't we drop the #ifdef for stat/win32_stat in file.c at the same time? Other than that, this starts to look ok now. // Martin _______________________________________________ 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".