[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: master e9e807e: Don't remove notify descriptor that is already gone
From: |
Michael Albinus |
Subject: |
Re: master e9e807e: Don't remove notify descriptor that is already gone |
Date: |
Fri, 19 Apr 2019 16:56:55 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) |
Mattias Engdegård <address@hidden> writes:
Hi Mattias,
>> I haven't tested thoroughly yet, but wouldn't it suffice if in
>> auto-revert-notify-rm-watch there is just the test
>>
>> (when (file-notify-valid-p auto-revert-notify-watch-descriptor)
>>
>> instead of
>>
>> (when auto-revert-notify-watch-descriptor
>
> Thanks for reading my change. It is a fair question!
>
> First of all, the descriptor wouldn't then be removed from
> `auto-revert-notify-watch-descriptor-hash-list' since that part is
> also guarded by the condition, but that's just a matter of rearranging
> code.
Yes.
> (Not only is `auto-revert-notify-watch-descriptor-hash-list' a
> mouthful, it is a bit misleading. It maps descriptors to lists of
> buffers. How about `auto-revert--buffers-by-watch-descriptor'?)
No objection.
> Slightly more robust would be to stop reusing descriptors: either made
> mutable, so that they can be invalidated, or made unique by using a
> counter. However, the basic design is still somewhat dubious: it tells
> us whether the descriptor is valid, but that just raises the question:
> why do we even have to ask? Correct code should understand its own
> invariants.
In theory you are right. But I fear there could be situations where such
assumptions do ne keep. A double-check is OK.
> Now that you `mentioned auto-revert-notify-rm-watch', does it strike
> you as odd the way it does
>
> (maphash
> (lambda (key value)
> (when (equal key some-key)
> do-something))
> some-hashtable)
>
> instead of using the hash table directly? Suggested patch to fix this
> attached.
I'm still not convinced that we need REMOVE-DESCRIPTOR. We shall always
remove the descriptor, and assure, that no superfluous events are raised.
> By the way, why don't we give each buffer in auto-revert-mode a unique
> descriptor, so that the table just maps descriptors to buffers,
> instead of to lists of buffers? It would simplify the code in many
> places, and it cannot be that common to have multiple buffers for the
> same file that it warrants the descriptor-sharing optimisation.
Well, the descriptor is the one we get from filenotify. I don't believe
we shall do double housekeeping. Sounds to me error-prone.
Best regards, Michael.