From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Matt Armstrong Newsgroups: gmane.emacs.bugs Subject: bug#46397: 27.1; Cannot delete buffer pointing to a file in a path that includes a file Date: Wed, 10 Feb 2021 14:39:36 -0800 Message-ID: References: <87h7mllgin.fsf@nexoid.at> <83a6scj745.fsf@gnu.org> <39d0e035-27b6-e2bd-daa2-747dda2c1a35@cs.ucla.edu> <835z2ziu52.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="14372"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (darwin) Cc: gmatta@gmail.com, 46397@debbugs.gnu.org, Paul Eggert , craven@gmx.net To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Thu Feb 11 00:07:18 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 1l9yZq-0003fG-Go for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 11 Feb 2021 00:07:18 +0100 Original-Received: from localhost ([::1]:34818 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1l9yZp-00006q-IG for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 10 Feb 2021 18:07:17 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:34724) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1l9y9T-00087C-6i for bug-gnu-emacs@gnu.org; Wed, 10 Feb 2021 17:40:03 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:45810) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1l9y9S-0005lV-8Z for bug-gnu-emacs@gnu.org; Wed, 10 Feb 2021 17:40:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1l9y9S-00053b-5z for bug-gnu-emacs@gnu.org; Wed, 10 Feb 2021 17:40:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Matt Armstrong Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 10 Feb 2021 22:40:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 46397 X-GNU-PR-Package: emacs Original-Received: via spool by 46397-submit@debbugs.gnu.org id=B46397.161299679219420 (code B ref 46397); Wed, 10 Feb 2021 22:40:02 +0000 Original-Received: (at 46397) by debbugs.gnu.org; 10 Feb 2021 22:39:52 +0000 Original-Received: from localhost ([127.0.0.1]:57356 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1l9y9H-00053A-OV for submit@debbugs.gnu.org; Wed, 10 Feb 2021 17:39:52 -0500 Original-Received: from relay1-d.mail.gandi.net ([217.70.183.193]:36155) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1l9y9G-00052w-Bi for 46397@debbugs.gnu.org; Wed, 10 Feb 2021 17:39:51 -0500 X-Originating-IP: 24.113.169.116 Original-Received: from matts-mbp-2016.lan (24-113-169-116.wavecable.com [24.113.169.116]) (Authenticated sender: matt@rfc20.org) by relay1-d.mail.gandi.net (Postfix) with ESMTPSA id BDC37240006; Wed, 10 Feb 2021 22:39:40 +0000 (UTC) In-Reply-To: <835z2ziu52.fsf@gnu.org> (Eli Zaretskii's message of "Wed, 10 Feb 2021 21:45:45 +0200") 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:199798 Archived-At: Eli Zaretskii writes: >> Cc: craven@gmx.net, 46397@debbugs.gnu.org, Matt Armstrong >> From: Paul Eggert >> Date: Wed, 10 Feb 2021 11:23:46 -0800 >> >> --- a/src/filelock.c >> +++ b/src/filelock.c >> @@ -532,7 +532,7 @@ current_lock_owner (lock_info_type *owner, char *lfname) >> /* If nonexistent lock file, all is well; otherwise, got strange error. */ >> lfinfolen = read_lock_data (lfname, owner->user); >> if (lfinfolen < 0) >> - return errno == ENOENT ? 0 : errno; >> + return errno == ENOENT || errno == ENOTDIR ? 0 : errno; > > As I said, I don't think this is TRT. Why should we silently ignore > ENOTDIR? couldn't it mean some kind of malicious attack on the user's > account, or some other trouble? > > We should not ignore these errors, we should ask the user what to do > about them. The user can tell us the error can be ignored, but we > should not decide that without asking. I think Paul's commit is a good one. I'll try to explain why. The commit does not silently ignore ENOTDIR. Instead, it is explicitly handles that particular error code it in a way that honors the lock file API contract. In this case, Paul's commit changes the current_lock_owner() function such that it returns zero upon ENOTDIR. The caller must interpret the zero return as meaning "at the time current_lock_owner() was called, nobody owned the lock file...or the lock file was obsolete." ENOTDIR has a specific meaning that we can rely on. Both ENOENT and ENOTDIR imply that the file was definitely not on disk at the time of the call. Because of this, current_lock_owner() can safely conclude that nobody owned the lock. Put another way, I think a clearer API spec for current_lock_owner() would be: /* Return -2 if the current process owns the lock file, or -1 if another process appears to own it (and set OWNER (if non-null) to info), or 0 if the lock file isn't there, or it is there but has no current owner, or an errno value if something is wrong with the locking mechanism. */ That spec is semantically equivalent to the current one, but makes it more clear why ENOENT and ENOTDIR should cause the function to return zero. Those errno values do not imply that "something is wrong with the locking mechanism." Up a level of abstraction, the callers of the lock file API are still presented with coherent semantics, even if current_lock_owner() returns "lock file not owned" when the directory is missing. If the higher level code wants to acquire the lock, it will attempt that and get an error at that time. If the higher level code wants to abandon a buffer, it can just do that knowing that this process definitely doesn't own that lock. In neither case the user need not be interrogated, and if they do, it will be done at a time that makes much more sense to them -- when actually trying to acquire the lock.