On 29/10/14 03:37, Daiki Ueno wrote:
Jesper Fehrlund <jes...@prisjakt.nu> writes:
I'm no c expert by any means, but does this not run xstrdup on a
dangling pointer?
(since best_cp->msgid and best_cp->msgid_plural points to the same
address which has been released in remember_a_message)
Good point, thanks. That part is really tricky. In the code:
mp = remember_a_message (..., best_cp->msgid, ...);
if (mp != NULL && best_cp->msgid_plural != NULL)
remember_a_message_plural (..., best_cp->msgid_plural, ...);
If MP is non-NULL, that means best_cp->msgid is saved in MP, and
remember_a_message_plural can still refer to the address pointed by
best_cp->msgid.
Of course, it is an implementation detail and the confusing dangling
pointer should be avoided. I'm thinking to cleanup the code, also by
moving the code conversion stuff to earlier phase.
Regards,
--
Daiki Ueno
Hmm, are you sure?
The first time a message is encountered, I can see that this would be
the case.
But, if the message already exists, then this code is executed:
mp = message_list_search (mlp, msgctxt, msgid);
if (mp != NULL)
{
if (msgctxt != NULL)
free (msgctxt);
free (msgid);
for (i = 0; i < NFORMATS; i++)
is_format[i] = mp->is_format[i];
do_wrap = mp->do_wrap;
}
And, as far as I can tell, at this point mp contains a string that is
equal to msgid (before free) yes, but it's not the same pointer.
I suppose in the patch it should be fine to use mp->msgid instead of
best_cp->msgid_plural for the copy
Regards,
--
Jesper Fehrlund
