From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from rock.gnat.com (rock.gnat.com [IPv6:2620:20:4000:0:a9e:1ff:fe9b:1d1]) by sourceware.org (Postfix) with ESMTPS id 791ED3858C2F; Thu, 23 Jun 2022 06:26:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 791ED3858C2F Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 386341161A7; Thu, 23 Jun 2022 02:26:18 -0400 (EDT) X-Virus-Scanned: Debian amavisd-new at gnat.com Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id toVRvhWa1DXK; Thu, 23 Jun 2022 02:26:18 -0400 (EDT) Received: from free.home (tron.gnat.com [IPv6:2620:20:4000:0:46a8:42ff:fe0e:e294]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPS id BCBEB1161A2; Thu, 23 Jun 2022 02:26:17 -0400 (EDT) Received: from livre (livre.home [172.31.160.2]) by free.home (8.15.2/8.15.2) with ESMTPS id 25N6Q2EE755199 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 23 Jun 2022 03:26:03 -0300 From: Alexandre Oliva To: Jonathan Wakely Cc: Sebastian Huber , "libstdc++" , gcc Patches , RTEMS Subject: Re: [PATCH] libstdc++: testsuite: fs rename to self may fail Organization: Free thinker, does not speak for AdaCore References: <4fe20709-e617-7644-175c-bd49b52dc6c2@embedded-brains.de> Errors-To: aoliva@lxoliva.fsfla.org Date: Thu, 23 Jun 2022 03:26:02 -0300 In-Reply-To: (Jonathan Wakely's message of "Wed, 22 Jun 2022 11:41:14 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Scanned-By: MIMEDefang 2.84 X-Spam-Status: No, score=-6.3 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 23 Jun 2022 06:26:20 -0000 On Jun 22, 2022, Jonathan Wakely wrote: > "If the old argument and the new argument resolve to either the same > existing directory entry or different directory entries for the same > existing file, rename() shall return successfully and perform no other > action." and "If the link named by the new argument exists, it shall > be removed and old renamed to new." > Instead, the implementation of std::filesystem::rename should have a > special-case for rtems (and maybe other targets) that implements the > POSIX rename semantics if calling ::rename isn't good enough. Other POSIX requirements that make implementing them "interesting" when ::rename() doesn't are: - leaving "new" alone when the operation fails: implies permissions, same filesystem, etc must all be checked first, atomically, before removing the file, and if renaming somehow fails after that, "new" must be somehow brought back to existence - if "new" exists, it must keep on existing throughout the renaming process: implies temporary removal is not allowed, even if needed to to allow ::rename to succeed; they'd have to be part of the same filesystem transaction - even if both of the above could somehow be satisfied, there are all sorts of race conditions that could enable pre-check to succeed, but that, even the checks were implemented fully and perfectly, would still enable the rename to fail after removal of a preexisting new. Clearly something's gotta give. Since ensuring the existence of "new" throughout renaming is impossible if you can't rename to existing filenames, I propose giving that up, though it's a very useful property. I'd start by trying to rename old to new. If that fails with EEXIST, I'd rename new to a temporary name, and then try to rename old to new and then remove the temporary name. If renaming new to a temporary name fails, that means we can't remove it either. If renaming old to the now-vacated new still fails, that means we can't rename old, so rename the temporary back to new. If removing the temporary name for new fails, that means we couldn't remove it to begin with, say because it's not empty, or because we lack permission to remove it, so rename both back. Knowing rename won't overwrite an existing pathname makes it easier to pick a temporary name to use for the renaming: we know there's no risk of overwriting files created concurrently, though we may have to try other names if the rename-to-temp fails with EEXIST. Creating a temporary directory sibling to new and picking a temporary name in that directory enables us to detect early some cases of lack of permission to remove new, that renaming it in the same directory wouldn't hit. Creating such a temporary directory also increases new/..'s link count before moving it. In a way, it checks for room for old/.., and it's no worse than keeping new under the same parent, but picking a temporary dir elsewhere would enable the renaming even if new/.. is already at the limit. Concurrent attempts to create directories in the same parent dir could enable the operation to fail while also preventing the reversal of the preparatory renames. That would be a rarer occurrence if we rename new to a temporary name under the same parent. And all this doesn't even cover the case of moving old into itself, which we'd also have to detect and prevent. This feels more and more like a case for xfail until it gets fixed in the kernel, where atomic filesystem operations belong :-( Would a patch to add: // { dg-xfail-if "::rename is not POSIX-compliant" { target *-*-rtems* } } to rename.cc tests be acceptable? I'm afraid I can't go further down this rabbit hole, and my choices ATM seem to be limited to XFAIL patches, whether accepted by the GCC community or carried internally. -- Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer Disinformation flourishes because many people care deeply about injustice but very few check the facts. Ask me about