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 0F1614117D for ; Mon, 16 May 2022 21:14:27 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 5370668B3BB; Tue, 17 May 2022 00:14:25 +0300 (EEST) Received: from NAM11-BN8-obe.outbound.protection.outlook.com (mail-bn8nam11olkn2093.outbound.protection.outlook.com [40.92.20.93]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id EFD1A68818E for ; Tue, 17 May 2022 00:14:17 +0300 (EEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=nhjwHA9dbW8rZEvDuuYFvvwl+s7g9YsQZguAHEqWThcnwAzRkD9WaFp0ORhmXQQrecbcsNrBr4WO90tIcWg241dE9JoeP6StG8KbiTvOuSwgxLNt0Tkdl21miYaS6/M5olqAEwoVkU4WNxActTNOuAL9Q2RcxCUjzfRbDAh3j4WtO2FAMtCnsN09wZvLN8i/m/WdiPnCVkqVmGdejV4KQxn3aRkBazN0ZKyMx8L0rglJrIOuX7FJgrIlVHnaNjqiYbe9J9qezt0sqGwJ1rDxkQA5p/Qgv7kCJWc1UGciE771mz4s8CXfmSPNdALl9Ksu3fU1+n/jVjhDRk+ad3RitQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=NqQo/5E377COBGEFSxv30hjvFuKwuQwemWPArik3t40=; b=mQSH9y9LSubfHmmZrPXhFuMTaIBoApjenm3YIn64S6DmH1ROI+/F03o8nV+TdyN3YdsQbjexSB/D1tpH1CDlg4vRMI2QmcGEslrjoWFhIZC3je7Cz/81DbEh83BCIQ7fIjTzdt7VXDXZ3KhQEwcc9jmRf+7CQf7mHtM0hPFACJcFvmnYIqvSiWdrnymfqcYELrtJEUQwSrgL9Ta6cioFPoAMBFigtoh5gY+g1cyg29hUdA2b7SlaADHTE0uRQQ1kD1lcl3WrG2/vClAqK1mmmU5JECgiAyKUE5/CVRlClq+mt8gJgfC/ljg5I9/RQUGPQKFSLUQzljAyVwAtX7E0Tw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=hotmail.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=NqQo/5E377COBGEFSxv30hjvFuKwuQwemWPArik3t40=; b=U0AvynGT4leTNNNgKDDoZIOJKFgIhZbAFyxdSwaKsfrhOXQbtwfMY7re8BscLZXxGZFHR5UAmN50GAbtWHGVSxAy61NHVcI4Rh/5xHfG3YZuDJ7shW1kUieXtNkZ04dvtY3S9YoHfBHhDJy7R8QMY0bxbHlVxFyLNJK3j5iwX6wYO3TzVRyRMJLW/DFnOVVBsw/ivOoZ+k/sATQhXL0t0+/6XOintsi7qH7Gq0dw3KA8dIGUWReVspiZEEr0qv2+VFaZhWdkGSEAxUwjCQAU9tcsmubxaKVr2El4NLfe9mf0AsdU2JK2rURap4RV8UJDiD7yuxXv84kuam0BYVjLAQ== Received: from DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM (2603:10b6:8:b::20) by DS7P223MB0526.NAMP223.PROD.OUTLOOK.COM (2603:10b6:8:9a::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5250.18; Mon, 16 May 2022 21:14:15 +0000 Received: from DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM ([fe80::c536:493f:7cda:53dc]) by DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM ([fe80::c536:493f:7cda:53dc%3]) with mapi id 15.20.5250.018; Mon, 16 May 2022 21:14:15 +0000 From: Soft Works To: FFmpeg development discussions and patches Thread-Topic: [FFmpeg-devel] [PATCH v2 1/2] avutil/wchar_filename, file_open: Support long file names on Windows Thread-Index: AQHYaPywrYyA1nT6+EGODV6JkiNpta0h8QeQ Date: Mon, 16 May 2022 21:14:14 +0000 Message-ID: References: In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-tmn: [V7hu1NoSCd3T9XbBTYdSCLQGFQyE7IYl] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: d1416050-143e-420d-13b8-08da378107f3 x-ms-traffictypediagnostic: DS7P223MB0526:EE_ x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: UeAQJV25M/RSYLy0Nu8g+WHU6AW9gTJWPt7mkw4xbA8oMA12AcyuMysEuK7Emu9fnXetV7Gh1AA3lAkrDRWQ2sWwtUFf6qaCf153YR3hg6DIa35KWryMokU0j//EBqLKqPr8Cc6K4L1zKZUgtBpsKLpwOQO5PJaReCFPMPpLYuSjb3gH64hp3M/bpVI25WyE8no8K4xAIczjz5CXn64+ImUcQt9YALApOjCn7O+vkLs+th3n9nrPL11Q/LuvO/LylIXyoIBpmj8Ll4Ui95WP5a/LPXMLBY702esNgAkW7VZ9MxwLpD4vc0B2YOF+0MkObpeRKZi3tFiclRdKY+GwZERPq8N3bqh2Jq1c3/4Knd7+WTrrtnIlAQDvqRhqUsWuagCX92b5LHtxDKSp4sQ/0d9gFgNosjqli64hmkTdHTa85xX5HEPyRjEOfD7LsTm/erseA0QUpw/1K2KLv+dEzM+ex7a1BjOxfRHv+bqao3tdGGrHh2HAzzoDNmEgjioCYe3mSVpGJJACoJZ4z3Yx4TH+Gfkwe4pVqYPBOtwT2YQ/gOw+ucp7MMXv0WFUQReLYeYwlPhEDG+zzj1/lcxZTQ== x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?utf-8?B?bzd4Sml1eEo5d0NCcE11R2kvWG5mU2djbXNzOTZCNmQwTkQ3VHZCSHBzL292?= =?utf-8?B?RHAwTjRiNzJiUnNzRkpEeTlwQm80MVRrTnRUZWJIV3NnTlNINWpXakdjT05R?= =?utf-8?B?TFJuT0I4M081dlJuUmRuVlEyK1dHa3dXVzZrMU5jRTVhakx0RzBDdXJ3c2po?= =?utf-8?B?QUczRjFEOFZ3Myt2UnpRRE1NNElJbUZmR2RobVlSSW1rVGJuWU5SQSthblJm?= =?utf-8?B?MllaaHgyazErMzJhTU1sNG01VnVldlhlYkZMN21jbitaS3Q3MnhpMUVhbHRS?= =?utf-8?B?aUxCT3BobVdoOEZlSytjYklzQkxUVFBDZkNaZ2NBejBEdDQzY3k0c3ZwZkFH?= =?utf-8?B?VlVLQnI4TG84RWNQMXVrL2FKY1RPQStWZ1I4UVdtRWZJZGFJazc0VTA4STY0?= =?utf-8?B?WjZscHR0M0hhQ201V3FiS2pnSXZFcEt0M0s4ZXFjSCtwd3pVcTZMM3BEOEdH?= =?utf-8?B?bUVvT0tRUTQ2a3BKRHdmcGNJOHNKRzhZNjlUbFJvUnlXS2xXR0UzZVF1anJJ?= =?utf-8?B?cFJhWlBVaWVLZUtuQ3hMOS9ORG15c0cwWDNJenZ5V3Ewd1ZkYjk5YnkzaVZh?= =?utf-8?B?Y0hicTFXTzlObG5CQ08zdXpnUW9TcW1CNUkzNW1WSzh1Vm9udUVPb1UydjBm?= =?utf-8?B?YXRtRTRvNDFoSUlUaXAxWWN0czI0WXdEVXBiQ1dYOTg4SC9vaHJBNytsc1Mv?= =?utf-8?B?aStaTTJVRmhDUVdHR2dmZ2hxUzIrNmplZEFEdTEvb3RqN2ppNDdMTkc1ZnFI?= =?utf-8?B?WEhDUXY0bUFxOFIxbFg2T3dzK1U2NHJyS0pLdE9HdjlLZkd1R1FFNC9kaVZx?= =?utf-8?B?cHdNcW11OG9RMEdBSlU3OXZoeU9lUS9yR1RBSGNOcUJ3ZGIwYS8wc0JTcVVJ?= =?utf-8?B?clVtdGNDRzVpU1lMS0hxcFBHMjYxdzNVOVlPdXpIMFdpQUMwUFZBd0VNTnJG?= =?utf-8?B?YnNKUnNDb0dFMG5SRmlBdHVNMEptVHUxTUxnbUh0d3lRQ0lhb0d4eitiV004?= =?utf-8?B?MVAyMm9PdEpUQ0grVkhnWHFTVWFrSjdsVXZibGV3Vm1rNE9yZ0luVHRVa25Q?= =?utf-8?B?R09NeHh2Y2tER3UxUlkzUCsyckhLeDVJVFQ4NXdNTnVSMlFWUGkyUThrQllJ?= =?utf-8?B?SCttbG5aUHRRYkZjOHZDT1dJS3RPbFM0d2lDc2RmOTRJUHQ0dHNpV0pQOUlS?= =?utf-8?B?QUNqTTZ1WjhMWXovQklLeVpxZzFPRXJOYitVZEd5UlFkVk1LQmRadHBscHJ6?= =?utf-8?B?c1hadmVYcS81TUNOV3FwRjlQeS8xdzZaNWcvcDhsZVJQRTNTTUJ1VExiYzly?= =?utf-8?B?T1JLRDVMN2gwdXg0TmFxQm5KVlMra3RCVmJSazFrWlpLdnRMUnp0L0JsWVZD?= =?utf-8?B?Q2Q3VjlERzJnRnNkZmtzbHF0RUpwLzh0MnlwTU44MmM1NndtRDl6TUtSZGhx?= =?utf-8?B?VWRsSkQraHhzNC8wc21DaS9BeWhiV0d6MG5CSE10Y01JU2loVUg5YnhFSzdV?= =?utf-8?B?RGZkVnhyeVpjTkdrdDRYMVRWdWFIRm9NVDZLSVZXNzRTc1piVmpaRGdETlho?= =?utf-8?B?OWh0WHFBY3Rub21wWGFHN1VlbGoxWU1jdkwyd1pmQjZMbGZaWFNIbVJnNlhz?= =?utf-8?B?S3g5VVE3TFlVL1VIM09Gblc4SEg3Wlpxb25pY2ZDeTJuQ3FKSktqdXJwODFr?= =?utf-8?B?eVV2eSt0VDl2ZlVOYjg4aittV3JVRko5Q25nSUcya3IzRzNySjE2aXhMeDZW?= =?utf-8?B?Vm1mSlVmQ2djb1J6L3hUV2I2OG9nd1ZNUWZURTkwRFdCcThHRVdkLzhjaTNa?= =?utf-8?B?c3hIRkhTejBzQWcrRXpZSU43OGFuaEthdUZTQmRMKzFaSHNkbzhIUmdhay9X?= =?utf-8?B?dFpSZm44UGpyM1pMWWZ4dFQrbWpyRUZjVWM5Yng1dldUK3EwZE1rWStYQmxS?= =?utf-8?Q?m9HvUw4M/Oo=3D?= MIME-Version: 1.0 X-OriginatorOrg: sct-15-20-4755-11-msonline-outlook-1ff67.templateTenant X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-CrossTenant-Network-Message-Id: d1416050-143e-420d-13b8-08da378107f3 X-MS-Exchange-CrossTenant-originalarrivaltime: 16 May 2022 21:14:14.9869 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7P223MB0526 Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] avutil/wchar_filename, file_open: 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 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: > -----Original Message----- > From: ffmpeg-devel On Behalf Of nil- > admirari@mailo.com > Sent: Monday, May 16, 2022 10:12 AM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] avutil/wchar_filename, > file_open: Support long file names on Windows > > > +static inline int path_is_extended(const wchar_t *path) > > +{ > > + size_t len = wcslen(path); > > + if (len >= 4 && path[0] == L'\\' && (path[1] == L'\\' || path[1] > == L'?') && path[2] == L'?' && path[3] == L'\\') > > Length check is probably unnecessary: comparisons will reject '\0' > and further comparisons won't run due to short-circuiting. Yup, I think you're right, even though it would appear "unsafe" at first sight. I've removed the length check. > > + // The length of unc_prefix is 6 plus 1 for terminating zeros > > + temp_w = (wchar_t *)av_calloc(len + 6 + 1, sizeof(wchar_t)); > > Not really true. The length of unc_prefix is 8. > 2 is subtracted because UNC path already has \\ at the beginning. Correct. It actually needs to be "len - 2 + 8 + 1". I've updated the comment and the calculation. > > + if (len >= 260 || (*ppath_w)[len - 1] == L' ' || (*ppath_w)[len - > 1] == L'.') { > > 1. Please change 260 to MAX_PATH. Done. > 2. GetFullPathName removes trailing spaces and dots, so the second > part is always false. Yea, when someone would want to handle such (weird) kind of path, one would need to specify an extended path directly. Removed the second part. > > 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. stat wasn't already defined as win32_stat. win32_stat was already defined but not mapped. That's what my change does. > > The functions are needed in both. file_open.c cannot be included > > in libavformat/os_support.h and neither the other way round, > > so they need to be in a 3rd place. How about renaming > > wchar_filename.h to windows_filename.h ? > > Probably it's better to rename. OK, but let's do this in subsequent patch shortly after. > > I have skipped those checks because we won't have partially > qualified > > paths at this point (due to having called GetFullPathNameW) and > > device paths are not allowed to be longer than 260, so this it might > > happen that the UNC prefix gets added, but only when it's a long > > path which doesn't work anyway (I've tested those cases). > > I think it's better to test for \\.\ explicitly in path_is_extended: > 1. It's not obvious that \\.\ aren't allowed to be long. > 2. Probably FFmpeg is not going to have a longPathAware manifest, > but it can be linked with an EXE with such a manifest. > Would MAX_PATH restriction still apply? That's a good question, but we need to be clear that device paths are actually intended for accessing devices, e.g. like \\.\COM1 The fact that the prefix also works with a drive-letter path is more due to some heritage and nobody would normally want to use such kind of paths to access files. That being said, I've added a check (path_is_device_path()) for this now in the same way as .NET is doing it - which means inside add_extended_prefix(). > You have the checks inside of get_extended_win32_path and none > inside of add_extended_prefix. Yet add_extended_prefix can be called > by anyone: it's not private. Thus add_extended_prefix either should be > inlined, > or it should have the necessary checks in place. Otherwise you end up > with > an API that's easy to use incorrectly and hard to use correctly, and > it should be the other way around. I have added checks there now for both device path and extended path prefix. I have also clarified the function doc even further, so I hope it's sufficiently clear now. ;-) > > And what's the point about this? > > Point is obvious: extended paths are difficult to handle correctly. > get_extended_win32_path cannot be used on its own, only as a final > step before getting FILE* or a file descriptor. Yes, that's how it's meant to be used, so the whole application can be kept free from dealing with it. Thanks again for the review, these were some good improvements. 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".