[Dovecot] suspect valgrind error in mail-index-map.c
Hi
At line 1118 of src//lib-index/mail-index-map.c, inside the function mail_index_map_move_to_memory, there is: mail_index_map_copy_header(map, map);
Valgrind is stating that "Source and destination overlap in memcpy". I'm wondering if this code is just coping the same memory over itself, or if it is doing something useful.
Regards, Diego Liziero.
1104 void mail_index_map_move_to_memory(struct mail_index_map *map) 1105 { 1106 struct mail_index_record_map *new_map; 1107 1108 if (map->rec_map->mmap_base == NULL) 1109 return; 1110 1111 i_assert(map->rec_map->lock_id != 0); 1112 1113 new_map = array_count(&map->rec_map->maps) == 1 ? map->rec_map : 1114 mail_index_record_map_alloc(map); 1115 1116 mail_index_map_copy_records(new_map, map->rec_map, 1117 map->hdr.record_size); 1118 mail_index_map_copy_header(map, map); 1119 1120 if (new_map != map->rec_map) { 1121 mail_index_record_map_unlink(map); 1122 map->rec_map = new_map; 1123 } else { 1124 mail_index_unlock(map->index, &new_map->lock_id); 1125 if (munmap(new_map->mmap_base, new_map->mmap_size) < 0) 1126 mail_index_set_syscall_error(map->index, "munmap()"); 1127 new_map->mmap_base = NULL; 1128 } 1129 }
On Sat, 2008-03-08 at 17:26 +0100, Diego Liziero wrote:
Valgrind is stating that "Source and destination overlap in memcpy". I'm wondering if this code is just coping the same memory over itself, or if it is doing something useful.
It's copying over itself, so it shouldn't break anything. But I fixed the error anyway: http://hg.dovecot.org/dovecot-1.1/rev/8e014fd46e84
On Sun, Mar 9, 2008 at 1:12 AM, Timo Sirainen tss@iki.fi wrote:
It's copying over itself, so it shouldn't break anything. But I fixed the error anyway: http://hg.dovecot.org/dovecot-1.1/rev/8e014fd46e84
Thanks Timo. With this patch valgrind gives 0 errors most of the time.
Forgive me if I'm asking this last valgrind trace. Can we simply ignore it?
Diego.
==3921== 180 (124 direct, 56 indirect) bytes in 1 blocks are definitely lost in loss record 3 of 5 ==3921== at 0x40046FF: calloc (vg_replace_malloc.c:279) ==3921== by 0x80D76DC: pool_system_malloc (mempool-system.c:74) ==3921== by 0x80B59CB: mail_transaction_log_file_alloc (mail-transaction-log-file.c:51) ==3921== by 0x80B3A86: mail_transaction_log_find_file (mail-transaction-log.c:385) ==3921== by 0x80B776F: mail_transaction_log_view_set (mail-transaction-log-view.c:160) ==3921== by 0x80B00B2: mail_index_sync_map (mail-index-sync-update.c:747) ==3921== by 0x80A9368: mail_index_map (mail-index-map.c:897) ==3921== by 0x80A63BB: mail_index_try_open (mail-index.c:290) ==3921== by 0x80A673C: mail_index_open (mail-index.c:352) ==3921== by 0x809AB78: index_storage_mailbox_open (index-storage.c:383) ==3921== by 0x807809B: mbox_alloc_mailbox (mbox-storage.c:573) ==3921== by 0x8078E2F: mbox_open (mbox-storage.c:591) ==3921== by 0x807900E: mbox_mailbox_open (mbox-storage.c:668) ==3921== by 0x809EB98: mailbox_open (mail-storage.c:459) ==3921== by 0x805E0E8: cmd_select_full (cmd-select.c:32) ==3921== by 0x805B5A8: cmd_examine (cmd-examine.c:8) ==3921== by 0x805F9A8: client_command_input (client.c:546) ==3921== by 0x805FA3D: client_command_input (client.c:595) ==3921== by 0x80601C4: client_handle_input (client.c:636) ==3921== by 0x80603DD: client_input (client.c:691) ==3921== by 0x80D5B9F: io_loop_handler_run (ioloop-epoll.c:201) ==3921== by 0x80D4E27: io_loop_run (ioloop.c:301) ==3921== by 0x8067DBB: main (main.c:293)
On Sun, Mar 9, 2008 at 2:07 AM, Diego Liziero diegoliz@gmail.com wrote:
[..]
180 (124 direct, 56 indirect) bytes in 1 blocks are definitely lost in loss record 3 of 5 [..] by 0x80B59CB: mail_transaction_log_file_alloc (mail-transaction-log-file.c:51) by 0x80B3A86: mail_transaction_log_find_file (mail-transaction-log.c:385)
I think that it's this last line causing the error.
---- mail-transaction-log.c 343 int mail_transaction_log_find_file(struct mail_transaction_log *log, 344 uint32_t file_seq, bool nfs_flush, 345 struct mail_transaction_log_file **file_r) 346 { 347 struct mail_transaction_log_file *file; 348 const char *path; 349 int ret; [..] 382 /* see if we have it in log.2 file */ 383 path = t_strconcat(log->index->filepath, 384 MAIL_TRANSACTION_LOG_SUFFIX".2", NULL); 385 file = mail_transaction_log_file_alloc(log, path);
Here a new mail_transaction_log_file is allocated before getting lost. Maybe I'm wrong, but, isn't here a path where mail_transaction_log_file_free(&file); should be called before returning without losing the memory pointed by file?
386 if ((ret = mail_transaction_log_file_open(file, TRUE)) <= 0) 387 return ret; 388 389 /* but is it what we expected? */ 390 if (file->hdr.file_seq != file_seq) 391 return 0; 392 393 *file_r = file; 394 return 1; 395 }
Regards, Diego.
On Sun, 2008-03-09 at 21:16 +0100, Diego Liziero wrote:
385 file = mail_transaction_log_file_alloc(log, path);
Here a new mail_transaction_log_file is allocated before getting lost. Maybe I'm wrong, but, isn't here a path where mail_transaction_log_file_free(&file); should be called before returning without losing the memory pointed by file?
Right. Thanks, fixed: http://hg.dovecot.org/dovecot-1.1/rev/e569788da4e8
participants (2)
-
Diego Liziero
-
Timo Sirainen