Mhhh... This patch looks like it gets a couple of things right,
hence I'm committing it. If it breaks things, we can still back out.
--
Thomas Roessler · Personal soap box at <http://log.does-not-exist.org/>.
On 2005-05-25 18:39:05 +0200, Thomas Glanzmann wrote:
> From: Thomas Glanzmann <sithglan@xxxxxxxxxxxxxxxxxxxx>
> To: Alex Muntada <alexm@xxxxxxxxx>, Mutt Developers <mutt-dev@xxxxxxxx>,
> Brendan Cully <brendan@xxxxxxxxxx>,
> Thomas Roessler <roessler@xxxxxxxxxxxxxxxxxx>
> Date: Wed, 25 May 2005 18:39:05 +0200
> Subject: Re: Fwd: Re: Segmentation fault
> Mail-Followup-To: Alex Muntada <alexm@xxxxxxxxx>,
> Mutt Developers <mutt-dev@xxxxxxxx>,
> Brendan Cully <brendan@xxxxxxxxxx>,
> Thomas Roessler <roessler@xxxxxxxxxxxxxxxxxx>
> X-Spam-Level:
>
> Hello Alex,
>
> ==23030== Invalid write of size 4
> ==23030== at 0x808EA84: mx_update_context (mx.c:1581)
> ==23030== by 0x80D677C: imap_read_headers (message.c:317)
> ==23030== by 0x80D27F0: imap_cmd_finish (command.c:266)
> ==23030== by 0x80D53D7: imap_check_mailbox (imap.c:1181)
> ==23030== by 0x808E53F: mx_check_mailbox (mx.c:1365)
> ==23030== by 0x806467E: mutt_index_menu (curs_main.c:479)
> ==23030== by 0x8082154: main (main.c:934)
> ==23030== Address 0x1BE58E40 is 0 bytes after a block of size 1480 alloc'd
> ==23030== at 0x1B902A2F: realloc (vg_replace_malloc.c:197)
> ==23030== by 0x80B9467: safe_realloc (lib.c:109)
> ==23030== by 0x808E95A: mx_alloc_memory (mx.c:1547)
> ==23030== by 0x80D61D3: imap_read_headers (message.c:118)
> ==23030== by 0x80D27F0: imap_cmd_finish (command.c:266)
> ==23030== by 0x80D53D7: imap_check_mailbox (imap.c:1181)
> ==23030== by 0x808E53F: mx_check_mailbox (mx.c:1365)
> ==23030== by 0x806467E: mutt_index_menu (curs_main.c:479)
> ==23030== by 0x8082154: main (main.c:934)
> ==23030== Warning: invalid file descriptor -1 in syscall close()
> ==3476==
>
> Brendan, can you say something to this one? It seems that there isn't enough
> space allocated to handle all the headers. I included a maybe work
> around in my patch.
>
> ==4365== Invalid read of size 1
> ==4365== at 0x1B901A60: strcmp (mac_replace_strmem.c:249)
> ==4365== by 0x80BA2AC: mutt_strcmp (lib.c:637)
> ==4365== by 0x807A4F2: hash_delete_hash (hash.c:127)
>
> ==4365== Invalid read of size 1
> ==4365== at 0x1B901A63: strcmp (mac_replace_strmem.c:250)
> ==4365== by 0x80BA2AC: mutt_strcmp (lib.c:637)
> ==4365== by 0x807A474: hash_find_hash (hash.c:108)
>
> these two come from my faulty patch, I guess. So here is another patch which
> hopefully doesn't break anything. I tried a much simpler approch. However I
> still don't see how the old one was broke. Apply diff against CVS HEAD;
> or delta against your current tree.
>
> Sorting mailbox...
> Program received signal SIGSEGV, Segmentation fault.
> 0x080b738f in mutt_sort_threads (ctx=0x8103d90, init=0) at thread.c:859
> 859 tmp = new->next;
> (gdb) p new
> $1 = (THREAD *) 0x162
> (gdb) i s
> #0 0x080b738f in mutt_sort_threads (ctx=0x8103d90, init=0) at thread.c:859
> #1 0x080b4561 in mutt_sort_headers (ctx=0x8103d90, init=0) at sort.c:291
> #2 0x080640b7 in update_index (menu=0x8172740, ctx=0x8103d90, check=1,
> oldcount=343, index_hint=330) at curs_main.c:317
> #3 0x08064712 in mutt_index_menu () at curs_main.c:492
> #4 0x08082155 in main (argc=1, argv=0xbfffe404) at main.c:934
> #5 0x42017589 in __libc_start_main () from /lib/i686/libc.so.6
> (gdb) p thread
> $2 = (THREAD *) 0x8126e70
> (gdb) p *thread
> $3 = {fake_thread = 0, duplicate_thread = 0, sort_children = 0,
> check_subject = 0, visible = 0, deep = 1, subtree_visible = 1,
> next_subtree_visible = 1, parent = 0xbfffd420, child = 0x162,
> next = 0x8126e90, prev = 0x8129b50, message = 0x81688a0,
> sort_key = 0x81688a0}
> (gdb) p tmp
> $4 = (THREAD *) 0x0
>
> no idea where this comes from.
>
> Thomas
> diff --git a/hash.c b/hash.c
> --- a/hash.c
> +++ b/hash.c
> @@ -114,8 +114,12 @@ void *hash_find_hash (const HASH * table
> void hash_delete_hash (HASH * table, int hash, const char *key, const void
> *data,
> void (*destroy) (void *))
> {
> - struct hash_elem *ptr = table->table[hash];
> - struct hash_elem **last = &table->table[hash];
> + struct hash_elem *ptr;
> + struct hash_elem **last;
> +
> +again:
> + ptr = table->table[hash];
> + last = &table->table[hash];
>
> for (; ptr; last = &ptr->next, ptr = ptr->next)
> {
> @@ -128,7 +132,7 @@ void hash_delete_hash (HASH * table, int
> *last = ptr->next;
> if (destroy) destroy (ptr->data);
> FREE (&ptr);
> - return;
> + goto again;
> }
> }
> }
> diff --git a/imap/message.c b/imap/message.c
> --- a/imap/message.c
> +++ b/imap/message.c
> @@ -314,7 +314,10 @@ int imap_read_headers (IMAP_DATA* idata,
> fclose(fp);
>
> if (ctx->msgcount > oldmsgcount)
> + {
> + mx_alloc_memory(ctx);
> mx_update_context (ctx, ctx->msgcount - oldmsgcount);
> + }
>
> return msgend;
> }
> @@ -469,6 +472,8 @@ int imap_fetch_message (MESSAGE *msg, CO
> hash_delete (ctx->id_hash, h->env->message_id, h, NULL);
> if (ctx->subj_hash && h->env->real_subj)
> hash_delete (ctx->subj_hash, h->env->real_subj, h, NULL);
> + if (ctx->thread_hash && h->env->message_id)
> + hash_delete (ctx->thread_hash, h->env->message_id, NULL, NULL);
> mutt_free_envelope (&h->env);
> h->env = mutt_read_rfc822_header (msg->fp, h, 0, 0);
> if (ctx->id_hash && h->env->message_id)
> diff --git a/imap/util.c b/imap/util.c
> --- a/imap/util.c
> +++ b/imap/util.c
> @@ -138,11 +138,11 @@ int imap_parse_path (const char* path, I
> {
> *c = '\0';
> strfcpy (mx->account.user, tmp, sizeof (mx->account.user));
> - strfcpy (tmp, c+1, sizeof (tmp));
> + c++;
> mx->account.flags |= M_ACCT_USER;
> }
>
> - if ((n = sscanf (tmp, "%127[^:/]%127s", mx->account.host, tmp)) < 1)
> + if ((n = sscanf (c, "%127[^:/]%127s", mx->account.host, c)) < 1)
> {
> dprint (1, (debugfile, "imap_parse_path: NULL host in %s\n", path));
> FREE (&mx->mbox);
> @@ -150,11 +150,11 @@ int imap_parse_path (const char* path, I
> }
>
> if (n > 1) {
> - if (sscanf (tmp, ":%hu%127s", &(mx->account.port), tmp) >= 1)
> + if (sscanf (c, ":%hu%127s", &(mx->account.port), c) >= 1)
> mx->account.flags |= M_ACCT_PORT;
> - if (sscanf (tmp, "/%s", tmp) == 1)
> + if (sscanf (c, "/%s", c) == 1)
> {
> - if (!ascii_strncmp (tmp, "ssl", 3))
> + if (!ascii_strncmp (c, "ssl", 3))
> mx->account.flags |= M_ACCT_SSL;
> else
> {
> diff --git a/hash.c b/hash.c
> --- a/hash.c
> +++ b/hash.c
> @@ -114,8 +114,12 @@ void *hash_find_hash (const HASH * table
> void hash_delete_hash (HASH * table, int hash, const char *key, const void
> *data,
> void (*destroy) (void *))
> {
> - struct hash_elem *ptr = table->table[hash];
> - struct hash_elem **last = &table->table[hash];
> + struct hash_elem *ptr;
> + struct hash_elem **last;
> +
> +again:
> + ptr = table->table[hash];
> + last = &table->table[hash];
>
> for (; ptr; last = &ptr->next, ptr = ptr->next)
> {
> @@ -123,15 +127,11 @@ void hash_delete_hash (HASH * table, int
> * required for the case where we have multiple entries with the same
> * key
> */
> -again:
> if ((data == ptr->data) || (!data && mutt_strcmp (ptr->key, key) == 0))
> {
> *last = ptr->next;
> if (destroy) destroy (ptr->data);
> FREE (&ptr);
> - ptr = *last;
> - if (! ptr)
> - return;
> goto again;
> }
> }
> diff --git a/imap/message.c b/imap/message.c
> --- a/imap/message.c
> +++ b/imap/message.c
> @@ -314,7 +314,10 @@ int imap_read_headers (IMAP_DATA* idata,
> fclose(fp);
>
> if (ctx->msgcount > oldmsgcount)
> + {
> + mx_alloc_memory(ctx);
> mx_update_context (ctx, ctx->msgcount - oldmsgcount);
> + }
>
> return msgend;
> }
Attachment:
pgpcnsZcy0mrb.pgp
Description: PGP signature