From: Lars Ingebrigtsen <larsi@gnus.org>
To: Eli Zaretskii <eliz@gnu.org>
Cc: michael.albinus@gmx.de, ncaprisunfan@gmail.com, 49261@debbugs.gnu.org
Subject: bug#49261: 28.0.50; File Locking Breaks Presumptuous Toolchains
Date: Wed, 07 Jul 2021 20:17:22 +0200 [thread overview]
Message-ID: <87h7h6m1jh.fsf@gnus.org> (raw)
In-Reply-To: <834kd6f1dw.fsf@gnu.org> (Eli Zaretskii's message of "Wed, 07 Jul 2021 21:02:35 +0300")
Eli Zaretskii <eliz@gnu.org> 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
next prev parent reply other threads:[~2021-07-07 18:17 UTC|newest]
Thread overview: 109+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-28 17:38 bug#49261: 28.0.50; File Locking Breaks Presumptuous Toolchains Mallchad Skeghyeph
2021-06-30 13:00 ` Lars Ingebrigtsen
2021-06-30 13:26 ` Eli Zaretskii
2021-06-30 14:08 ` Lars Ingebrigtsen
[not found] ` <CADrO7Mje3DstmjxutZcpx33jWJwgE_z+hGfJc4aON1CYOpyJxA@mail.gmail.com>
2021-07-01 10:55 ` Lars Ingebrigtsen
2021-07-01 12:58 ` Eli Zaretskii
2021-06-30 16:07 ` Michael Albinus
2021-06-30 16:16 ` Eli Zaretskii
2021-07-01 11:38 ` Lars Ingebrigtsen
2021-06-30 19:31 ` Juri Linkov
2021-07-01 16:57 ` Michael Albinus
2021-07-01 18:31 ` Eli Zaretskii
2021-07-02 11:06 ` Lars Ingebrigtsen
2021-07-02 12:32 ` Michael Albinus
2021-07-07 16:01 ` Lars Ingebrigtsen
2021-07-07 16:07 ` Michael Albinus
2021-07-07 16:13 ` Lars Ingebrigtsen
2021-07-07 16:40 ` Michael Albinus
2021-07-07 16:57 ` Lars Ingebrigtsen
2021-07-07 16:55 ` Michael Albinus
2021-07-07 16:59 ` Lars Ingebrigtsen
2021-07-07 17:36 ` Michael Albinus
2021-07-07 18:08 ` Lars Ingebrigtsen
2021-07-07 18:33 ` Eli Zaretskii
2021-07-07 18:50 ` Lars Ingebrigtsen
2021-07-07 19:40 ` Lars Ingebrigtsen
2021-07-07 20:03 ` Michael Albinus
2021-07-08 6:03 ` Michael Albinus
2021-07-08 19:53 ` Michael Albinus
2021-07-09 6:30 ` Eli Zaretskii
2021-07-09 8:28 ` Michael Albinus
2021-07-09 10:45 ` Eli Zaretskii
2021-07-09 11:01 ` Michael Albinus
2021-07-09 16:31 ` Lars Ingebrigtsen
2021-07-12 13:53 ` Michael Albinus
2021-07-12 14:03 ` Eli Zaretskii
2021-07-12 14:37 ` Michael Albinus
2021-07-12 17:30 ` Eli Zaretskii
2021-07-12 17:35 ` Lars Ingebrigtsen
2021-07-12 17:38 ` Eli Zaretskii
2021-07-12 18:00 ` Michael Albinus
2021-07-12 18:25 ` Eli Zaretskii
2021-07-12 18:46 ` Michael Albinus
2021-07-12 19:04 ` Eli Zaretskii
2021-07-13 17:53 ` Michael Albinus
2021-07-13 16:30 ` Lars Ingebrigtsen
2021-07-13 16:31 ` Lars Ingebrigtsen
2021-07-13 16:41 ` Eli Zaretskii
2021-07-13 17:59 ` Michael Albinus
2021-07-13 19:00 ` Eli Zaretskii
2021-07-13 19:09 ` Lars Ingebrigtsen
2021-07-13 19:36 ` Michael Albinus
2021-07-13 17:55 ` Michael Albinus
2021-07-13 19:05 ` Lars Ingebrigtsen
2021-07-16 16:15 ` Michael Albinus
2021-07-17 14:06 ` Lars Ingebrigtsen
2021-07-07 20:05 ` Eli Zaretskii
2021-07-07 20:09 ` Lars Ingebrigtsen
2021-07-07 20:15 ` Eli Zaretskii
2021-07-07 20:10 ` Eli Zaretskii
2021-07-07 20:18 ` Lars Ingebrigtsen
2021-07-07 20:29 ` Lars Ingebrigtsen
2021-07-07 20:37 ` Lars Ingebrigtsen
2021-07-07 20:55 ` Lars Ingebrigtsen
2021-07-07 21:04 ` Lars Ingebrigtsen
2021-07-07 22:22 ` Lars Ingebrigtsen
2021-07-08 0:09 ` bug#49261: Segfault during loadup Lars Ingebrigtsen
2021-07-08 6:35 ` Eli Zaretskii
2021-07-08 12:51 ` Lars Ingebrigtsen
2021-07-11 8:36 ` Paul Eggert
2021-07-11 10:21 ` Eli Zaretskii
2021-07-11 15:25 ` Eli Zaretskii
2021-07-12 7:16 ` Paul Eggert
2021-07-12 12:07 ` Eli Zaretskii
2021-07-12 14:50 ` Paul Eggert
2021-07-12 14:56 ` Andreas Schwab
2021-07-12 15:54 ` Eli Zaretskii
2021-07-13 23:12 ` Paul Eggert
2021-07-14 7:42 ` Andreas Schwab
2021-07-14 22:04 ` Paul Eggert
2021-07-14 22:10 ` Andreas Schwab
2021-07-14 12:36 ` Eli Zaretskii
2021-07-14 22:24 ` Paul Eggert
2021-07-15 6:13 ` Eli Zaretskii
2021-07-11 11:32 ` Lars Ingebrigtsen
2021-07-08 6:15 ` bug#49261: 28.0.50; File Locking Breaks Presumptuous Toolchains Eli Zaretskii
2021-07-08 6:20 ` Eli Zaretskii
2021-07-08 12:44 ` Lars Ingebrigtsen
2021-07-08 13:11 ` Lars Ingebrigtsen
2021-07-08 13:13 ` Eli Zaretskii
2021-07-08 6:17 ` Eli Zaretskii
2021-07-08 12:42 ` Lars Ingebrigtsen
2021-07-08 12:49 ` Lars Ingebrigtsen
2021-07-08 13:16 ` Eli Zaretskii
2021-07-08 13:34 ` Lars Ingebrigtsen
2021-07-08 16:47 ` Eli Zaretskii
2021-07-10 16:25 ` Lars Ingebrigtsen
2021-07-10 17:04 ` Eli Zaretskii
2021-07-10 17:15 ` Lars Ingebrigtsen
2021-07-10 17:20 ` Eli Zaretskii
2021-07-07 18:02 ` Eli Zaretskii
2021-07-07 18:17 ` Lars Ingebrigtsen [this message]
2021-07-07 18:20 ` Lars Ingebrigtsen
2021-07-07 18:42 ` Eli Zaretskii
2021-07-07 18:58 ` Lars Ingebrigtsen
2021-07-07 19:03 ` Lars Ingebrigtsen
2021-07-07 19:20 ` Eli Zaretskii
2021-07-07 18:50 ` Eli Zaretskii
2021-07-07 19:22 ` Lars Ingebrigtsen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87h7h6m1jh.fsf@gnus.org \
--to=larsi@gnus.org \
--cc=49261@debbugs.gnu.org \
--cc=eliz@gnu.org \
--cc=michael.albinus@gmx.de \
--cc=ncaprisunfan@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).