Re: Subtle bugs in interval code.

[storm@cua.dk (Kim F. Storm)]

David Kastrup <dak@gnu.org> writes:

> storm@cua.dk (Kim F. Storm) writes:
>
>> Studying the code in interval.c for merge_properties and
>> intervals_equal, I noticed that they use Fmemq to search
>> for a given property on a plist.
>>
>> That's not a safe way to do that;
>>
>> E.g. consider a plist like this:
>>
>> (setq p '(a b c d))
>>
>> Now,
>>
>>  (plist-get p 'a) => b
>>  (plist-get p 'b) => nil
>>
>> whereas
>>
>>  (memq 'a p) => (a b c d)
>>  (memq 'b p) => (b c d)   !=   nil
>>
>> So due to the use of Fmemq, the interval code may wrongly assume that
>> the plist has a `b' member with a value of `c'.
>>
>> I'm not aware of any specific bugs related to this.
>>
>> Note that we cannot just use plist-get instead of memq, as we then
>> cannot differentiate between "property is on list with nil value"
>> and "property is not on list".
>
> One could check whether the length of the rest sequence is even.

Not really.  Consider:

(memq 'b '(a b b c)) => (b b c)

> The cleanest option might be to add an optional DEFAULT argument to
> plist-get, but that would require changes to all C level callers.

No good!

> Maybe one should add an explicit plist-get-default function that takes
> the default for non-existing elements from its third argument?

That might work if you can advice on a default value that will
never be a valid element value of any relevant plist...

A version of memq which skips every 2nd element would work always!

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk