From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Lars Ingebrigtsen Newsgroups: gmane.emacs.bugs Subject: bug#49261: 28.0.50; File Locking Breaks Presumptuous Toolchains Date: Wed, 07 Jul 2021 20:17:22 +0200 Message-ID: <87h7h6m1jh.fsf@gnus.org> References: <87o8bn7bie.fsf@gnus.org> <87zgv6vuon.fsf@gmx.de> <837di9lwbm.fsf@gnu.org> <87a6n5vuu4.fsf@gnus.org> <8735sqnmei.fsf@gnus.org> <834kd6f1dw.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="30573"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: michael.albinus@gmx.de, ncaprisunfan@gmail.com, 49261@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Wed Jul 07 20:20:35 2021 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1m1C9z-0007kb-Cm for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 07 Jul 2021 20:20:35 +0200 Original-Received: from localhost ([::1]:55614 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1m1C9y-0005TG-CK for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 07 Jul 2021 14:20:34 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:57218) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m1C7W-00017C-MU for bug-gnu-emacs@gnu.org; Wed, 07 Jul 2021 14:18:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:41879) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1m1C7W-00055B-El for bug-gnu-emacs@gnu.org; Wed, 07 Jul 2021 14:18:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1m1C7W-0006Sf-9f for bug-gnu-emacs@gnu.org; Wed, 07 Jul 2021 14:18:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Lars Ingebrigtsen Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 07 Jul 2021 18:18:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 49261 X-GNU-PR-Package: emacs Original-Received: via spool by 49261-submit@debbugs.gnu.org id=B49261.162568185524799 (code B ref 49261); Wed, 07 Jul 2021 18:18:02 +0000 Original-Received: (at 49261) by debbugs.gnu.org; 7 Jul 2021 18:17:35 +0000 Original-Received: from localhost ([127.0.0.1]:53425 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1m1C75-0006Rv-HQ for submit@debbugs.gnu.org; Wed, 07 Jul 2021 14:17:35 -0400 Original-Received: from quimby.gnus.org ([95.216.78.240]:54304) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1m1C73-0006Ri-Cl for 49261@debbugs.gnu.org; Wed, 07 Jul 2021 14:17:34 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnus.org; s=20200322; h=Content-Type:MIME-Version:Message-ID:In-Reply-To:Date: References:Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=52B7Sn6Q/ZIdvYClXhwS4LHosDkMzwRXjDTbfDgNOR0=; b=L2BDssLozLQYwch5LlGsMiCuox CRZh3C+XpjsJihasTq2QzlCQlrV302pQcQY4W/PMmhrCPP6WSuS0hRBHcVgVvPOEFo0DlsT0qn3xh TLVeupu/LrqeRfUzr8wbJ8kHLar2ACVB2u6RQnvZq2jZJT2UVrbtF50Sz9ebtPHoOjAI=; Original-Received: from cm-84.212.220.105.getinternet.no ([84.212.220.105] helo=elva) by quimby.gnus.org with esmtpsa (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1m1C6t-0000QD-MZ; Wed, 07 Jul 2021 20:17:26 +0200 Face: iVBORw0KGgoAAAANSUhEUgAAADAAAAAwBAMAAAClLOS0AAAABGdBTUEAALGPC/xhBQAAACBj SFJNAAB6JgAAgIQAAPoAAACA6AAAdTAAAOpgAAA6mAAAF3CculE8AAAAElBMVEUHBwcoLixKVlRR dISqraj////Hgq6aAAAAAWJLR0QF+G/pxwAAAAd0SU1FB+UHBxIDNl6fdXwAAAGkSURBVDjLrZNh koMgDIWJ7gFI5AASPUAVDlAl9z/TBkTFzu6/OtOOzTckeY9XY777wJf71a72jxoC0YA0ftbD0OFr sJ4+EXVgFzLjMFJbnkzf8zRNvIYHgGg7YPSIRMvymGBNRx5HYw3ZDwBoDcJzgjEh00lWRP+o4yyJ Z1lUCyLWNbVnL+UpAtBWUzlKEtGv9XTldBtmrUfZrrbNVqDgXd9tJQeOSV61PoPF+4h2q8A7MA2g +WxFC+B4kyHKfk80/iIuSWrv7FrMbSIN6Oy54SxDA0x3bxX2FjTrpvgfcBfomBuw9xfomW+33A7p /PWI8ryZeGYAi4oqUatureKwEFfBS/NQvJk2rWfgbLmPt4E8ciWbw6Wln3krQE1nvfIAoOHNoJul 3K4WGXUieCU5QV25IZhsdQit6isgxZ0UH/nQnQytuacB+ZE4GoJDmIZKc81ZUvQSvSYWkbMC1Bdf LInbxPTqd40oHygnQj9hhyji/J4zujeJ9hbJSSLWOK55n9PLfHKQmDgPDSEwD5frLiJUX1UD+3CG zVtVcPwr9JQtzX4BceY4qJAP7wIAAAAQZVhJZklJKgAIAAAAAAAAAAAAAACcPLkoAAAAJXRFWHRk YXRlOmNyZWF0ZQAyMDIxLTA3LTA3VDE4OjAzOjU0KzAwOjAwPOfSYwAAACV0RVh0ZGF0ZTptb2Rp ZnkAMjAyMS0wNy0wN1QxODowMzo1NCswMDowME26at8AAAAASUVORK5CYII= X-Now-Playing: Tuxedomoon's _Live At The Palms (1978)_: "Ballad of the Coalminer" In-Reply-To: <834kd6f1dw.fsf@gnu.org> (Eli Zaretskii's message of "Wed, 07 Jul 2021 21:02:35 +0300") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:209608 Archived-At: Eli Zaretskii writes: > If this is going to be called from C, it should probably use > save-match-data, because no one will expect that just modifying a file > from some Lisp program could clobber the match data. Right; now added. > Also, do we ever need this during loadup? Because before files.el is > loaded by loadup, this function will not be available, so > unconditionally calling it from C without protection, not even > safe_call or somesuch, is not safe. I'll try doing a "make bootstrap"... >> +static Lisp_Object >> +make_lock_file_name (Lisp_Object fn) >> +{ >> + return ENCODE_FILE (call1 (intern ("make-lock-file-name"), fn)); >> +} > > I'd prefer not to have a function return an encoded file name, it's > unusual and unexpected. It is better to leave that to the caller. > (And if you do that, the rationale for having a separate function for > this will all but disappear, I think.) Right. But it seemed like all the callers wanted an encoded file name here, so it was marginally cleaner. >> - fn = Fexpand_file_name (fn, Qnil); >> + fn = make_lock_file_name (Fexpand_file_name (fn, Qnil)); > > In the original code, 'fn' was an un-encoded file name, but your > changes made it encoded. Why not keep the code more similar by having > a separate variable with the encoded file name? E.g., this would > avoid potential trouble here: Yup; I've now rewritten this to not reuse variables in this way, because it was pretty confusing. (See the version of the patch I posted some minutes ago.) >> if (!NILP (subject_buf) >> && NILP (Fverify_visited_file_modtime (subject_buf)) >> && !NILP (Ffile_exists_p (fn)) <<<<<<<<<<<<<<<<<<<<<<<<<<<<<< > > Ffile_exists_p was passed an un-encoded file name in the original > code. It calls file handlers, and encodes local file names by itself, > so it is better to pass it un-encoded file names. [...] > Same problem here: 'filename' is now an encoded file name, so you call > report_file_errno with an encoded file name, which is a no-no. Right; I'll fix up the un-encoded/encoded file name confusion here. > Last, but not least: do we care that now locking a file will cons > strings, even with the default value of lock-file-name-transforms? > That sounds like we are punishing the vast majority of users for the > benefit of the few who need the opt-in behavior. Should we perhaps > avoid consing strings, or maybe even calling Lisp altogether, under > the default value of that option? Hm... I think for simplicity's sake, it makes sense to always call the Lisp code. Having two places where we insert ".#" into a file name just seems error prone, long term. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no