Re: [gmime-devel] g_mime_header_list_clear leaving a list in limbo

[stedfast comcast net]

Hi Ludo,

Hah, awesome. I haven't used Pascal in ~15 years ;-)

Thanks for the code snippet. Looks like you found the bug, too - that if (!last->next) check looks wrong (copy&paste bug from iter_first() looks like).

I've modified git master to use a proper list_is_empty() check, since that's all that code was *trying* to do.

If you are able to test git master, would you let me know if that fixes the issue? If not, I'll test this tomorrow at the office.

Jeff

P.S. are you writing the pascal bindings? If not, where are they? Would love to link to them from the README/website.

---

From: "Ludo Brands" <ludo brands free fr>
To: stedfast comcast net
Cc: gmime-devel-list gnome org
Sent: Monday, November 26, 2012 10:11:08 AM
Subject: Re: [gmime-devel] g_mime_header_list_clear leaving a list in limbo

Hi Jeff,
> Hi Ludo,
>
> Can you write a small test case for the g_mime_header_iter_remove()
> bug that you are seeing?
>

I'm using gmime from freepascal but the routine is straightforward ;)

procedure TMime.testlistbug;
var
   headers: PGMimeHeaderList;
   iter: GMimeHeaderIter;
   res: gboolean;
begin
   headers:=g_mime_header_list_new;
   g_mime_header_list_set(headers,'A','1');
   g_mime_header_list_set(headers,'B','2');
   // print out
   g_mime_header_list_get_iter(headers,@iter);
   if g_mime_header_iter_first(@iter) then
     writeln('First1: ',g_mime_header_iter_get_name(@iter)+':
'+g_mime_header_iter_get_value(@iter));
   if g_mime_header_iter_last(@iter) then
     writeln('Last1 : ',g_mime_header_iter_get_name(@iter)+':
'+g_mime_header_iter_get_value(@iter));
   //delete all
   g_mime_header_iter_first(@iter);
   while g_mime_header_iter_remove(@iter) do ;
   if g_mime_header_iter_first(@iter) then
     writeln('First2: ',g_mime_header_iter_get_name(@iter)+':
'+g_mime_header_iter_get_value(@iter));
   if g_mime_header_iter_last(@iter) then
     writeln('Last2 : ',g_mime_header_iter_get_name(@iter)+':
'+g_mime_header_iter_get_value(@iter));
end;

Just adding 2 headers to a new header_list and printing first and last.
Then remove all with g_mime_header_iter_remove until false.
g_mime_header_iter_first returns false (correct) on the empty list but
g_mime_header_iter_last(@iter) returns true and garbage is printed.

g_mime_header_iter_last does a

     last = (GMimeHeader *) iter->hdrlist->list.tailpred;
     if (!last->next)
         return FALSE;

what is tailpred->next pointing to?


> I've got a Linux VM set up at the office that I can try to debug on
> while I try to figure out what is
> wrong with my desktop at home.
>
> There's no need to check/update tailpred in list_unlink() because of
> the way the linked list is
> designed (using aliases). Of course, it requires gcc
> -fno-strict-aliasing to be used since
> gcc 4.4 or so.
>
> At some point I should re-work list.c to not require
> -fno-strict-aliasing (I've been working on other
> areas where I use similar aliasing tricks), but as long as
> -fno-strict-aliasing continues to work,
> I am not in a huge rush.
>
> For example, check this out:
>
> typedef struct _ListNode {
>     struct _ListNode *next;
>     int id;
> } ListNode;
>
> ListNode *list, *tail, *node;
> int i;
>
> list = NULL;
> tail = (ListNode *) &list;
>
> for (i = 0; i < 10; i++) {
>     node = malloc (sizeof (ListNode));
>     node->id = i;
>
>     tail->next = node;
>     tail = node;
> }
>
> tail->next = NULL;
>
> Do you see the simplicity there? It's beautiful! No need to check if
> tail is NULL in the first
> iteration of the loop because assigning to tail->next will assign to
> list (only works because
> ListNode.next is the first member of the struct).
>
> This is how list.c works as well, except that it's a doubly-linked list.
>
> Jeff
>
Thanks. The example is clear. The extension to double linked lists is
less obvious though.

Ludo