GNU bug report logs - #75648
Minor safety improvements to fns.c/eval.c

Previous Next

Full log

Message #23 received at 75648 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 75648 <at> debbugs.gnu.org
Subject: Re: bug#75648: Minor safety improvements to fns.c/eval.c
Date: Sat, 18 Jan 2025 16:44:12 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> @@ -2697,15 +2700,14 @@ plist_put (Lisp_Object plist, Lisp_Object prop, Lisp_Object val)
>>  	  return plist;
>>  	}
>>
>> -      prev = tail;
>> -      tail = XCDR (tail);
>> +      prev_cdr = tail = tail_cdr;;
>>      }
>>    CHECK_TYPE (NILP (tail), Qplistp, plist);
>>    Lisp_Object newcell
>> -    = Fcons (prop, Fcons (val, NILP (prev) ? plist : XCDR (XCDR (prev))));
>> -  if (NILP (prev))
>> +    = Fcons (prop, Fcons (val, NILP (prev_cdr) ? plist : XCDR (prev_cdr)));
>> +  if (NILP (prev_cdr))
>>      return newcell;
>> -  Fsetcdr (XCDR (prev), newcell);
>> +  Fsetcdr (prev_cdr, newcell);
>>    return plist;
>>  }
>
> This is fine by me, but I wonder whether we should replace the NILP
> tests with !CONSP, when we are about to take XCAR or XCDR?

(Response to the plist bug fix only, thus I reduced the CC list).

We should!  I missed that one.  Is it okay if I just use CONSP and
reverse the arguments? "CONSP (x) ? XCDR (x) : y" makes it a little more
obvious that the code is safe than "!CONSP (x) ? y : XCDR (x)", IMHO.

Pip

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.