bug#13743: 24.2.93; Segmentation fault when trying to [s]teal a file opened elsewhere

[Stefan Monnier <monnier@iro.umontreal.ca>]

>> turn on overwrite-mode and replace the char at point with itself:
>> sure enough the buffer is marked as modified.
> Since you know how things work internally in this case, I'm surprised
> that you bring this example.

Actually, I don't really know how overwrite-mode works (I remembering
seeing it some point, but that's about as much I know about it).

> By contrast, in overwrite-mode we actually make the change without
> trying to avoid it.

That's an internal implementation detail, IMO.

>> Along the same lines, try (setq t t) and watch how it complains that
>> we're trying to modify a read-only object, ...
> Feel free to fix this blunder.

;-)

>> > You can repeat the last 2 steps forever, the buffer always becomes
>> > modified.  I don't see how this could be anything but a bug.  Not a
>> > catastrophe, I agree, but a bug nonetheless.
>> add-text-property is a mutation operation, like setq.  Whether or not it
>> returns data about the "old state" doesn't make it less of
>> a side-effecting operation, in my eyes.
> If add-text-property was a black box,

Why should we not consider it as a black box?

> I might consider agreeing with you.  But since it isn't, and its
> algorithm is glaringly clear, I don't.

I see nowhere (other than in its implementation) something that might
lead one to think that it's clever.

> The algorithm clearly tries to avoid mutation when possible, it just
> didn't go far enough.

Indeed, we could go further and reduce the interval passed to
modify_region in the case where some of the leading/trailing part of the
text already has the requested property values.

All these are just optimizations (i.e. quality of implementation
details, lack of which is not a bug).

>> So, no I do not consider it to be a bug at all.
> Not even considering the fact that it causes redisplay do redundant
> work?  If so, we will have to agree to disagree.

No, not even considering it.

>> >> And I don't think it's an important one here, since (as Dmitry points
>> >> out) the likely most common case (of having `start' be right at the
>> >> beginning of an interval object) didn't work anyway
>> > It does work now.  More importantly, it fixed the original crash.
>> I suspect it only works around the crash by optimizing away the call
>> to modify_region in the particular case you're testing.
> So you think I should install the followup I showed earlier?

I'd leaning towards yes, although it's sufficiently ugly (and dangerous
since there's no reason to assume that it won't inf-loop) that I'd
rather not.

> Not with mmm-mode, it doesn't.  If you repeat the original recipe for
> the crash, putting a breakpoint in filelock.c where it calls
> ask-user-about-lock, and type 'p' to the first prompt, you will get a
> second prompt, triggered by jit-lock, which does use
> with-silent-modifications, AFAICS.  I didn't try to figure out why
> this happens.

Looks like a weird corner case, indeed.  Maybe some code run from
jit-lock ends up (re)setting byte-file-name (which is normally bound to
nil by with-silent-modifications).


        Stefan