Re: [PATCH] fix for Crash in mapcar1
Martin Buchholz <martin(a)xemacs.org> writes:
I noticed this code in mapcar1:
else if (STRINGP (seq))
{
Bufbyte *p = XSTRING_DATA (seq);
for (i = 0; i < leni; i++)
{
args[1] = make_char (charptr_emchar (p));
INC_CHARPTR (p);
result = Ffuncall (2, args);
if (vals) vals[gcpro1.nvars++] = result;
}
}
which, it seems to me, could crash if STRING_DATA gets relocated.
Congratulations! I think you've just diagnosed a long-standing source
of "mysterious" crashes in mapconcat. Copying the string's data out
of harm's way is probably a good solution, although not one I'm
entirely happy with. (I sometimes wish XEmacs could do less copying.)
else if (STRINGP (sequence))
{
/* SEQUENCE might be relocated during GC. */
Not SEQUENCE, but SEQUENCE's STRING_DATA. You should be very clear so
that a later fiddler doesn't cry "no it can't!" and "fix" the
code.
size_t string_length = XSTRING_LENGTH (sequence);
Bufbyte *p = alloca_array (Bufbyte, string_length);
memcpy (p, XSTRING_DATA (sequence), string_length);
This is probably philosophical, but shouldn't the result of
XSTRING_LENGTH should be assigned to a Bytecount?
for (i = 0; i < leni; i++)
{
args[1] = make_char (charptr_emchar (p));
INC_CHARPTR (p);
vals[gcpro1.nvars++] = Ffuncall (2, args);
}
}
or:
Bufbyte *end = p + string_length;
while (p < end)
{
...
}