ctx->vsize reset

[Vincent Lefevre]

While testing the new code related to limited view, I noticed
an old standing bug: ctx->vsize wasn't reset in this code.

I'm wondering whether there could be similar bugs in other parts
of the code, where vcount is reset to 0, but not vsize.

mbox.c has:

ctx->hdrmax = 0; /* force allocation of new headers */
ctx->msgcount = 0;
ctx->vcount = 0;
ctx->tagged = 0;
ctx->deleted = 0;
ctx->new = 0;
ctx->unread = 0;
ctx->flagged = 0;
ctx->changed = 0;
ctx->id_hash = NULL;
ctx->subj_hash = NULL;

but nothing related to vsize.

sort.c has "ctx->vcount = 0;" twice, but nothing about vsize.

I don't know whether this is correct.

The other files with /vcount *= *0/ seem OK.

[Kevin J. McCarthy]

Post by Vincent Lefevre
I'm wondering whether there could be similar bugs in other parts
of the code, where vcount is reset to 0, but not vsize.

Nice sleuthing!

Post by Vincent Lefevre
[...]
but nothing related to vsize.

Since this is a reset (reopen), I think you're right, and vsize should
be cleared here too.

Post by Vincent Lefevre
sort.c has "ctx->vcount = 0;" twice, but nothing about vsize.
I don't know whether this is correct.

The first time, I'm not sure about, but since ctx->msgcount is 0, it
would seem safe to reset vsize too.

I think the second time is okay. The routine is just resorting, and
updating the virtual and v2r fields with the assumption that the actual
visible headers hasn't changed.

[Vincent Lefevre]

Post by Kevin J. McCarthy

Post by Vincent Lefevre
I'm wondering whether there could be similar bugs in other parts
of the code, where vcount is reset to 0, but not vsize.

Nice sleuthing!

Post by Vincent Lefevre
[...]
but nothing related to vsize.

Since this is a reset (reopen), I think you're right, and vsize should
be cleared here too.

Done. I could check that the old value is at least obsolete.

It is not clear whether this is useful, but at least, if there is a
bug somewhere else, it will be easier to notice it. For instance,
if vsize is not recomputed at some place, I suppose that one will
get 0 instead of a wrong value that could remained unnoticed.

Post by Kevin J. McCarthy

Post by Vincent Lefevre
sort.c has "ctx->vcount = 0;" twice, but nothing about vsize.
I don't know whether this is correct.

The first time, I'm not sure about, but since ctx->msgcount is 0, it
would seem safe to reset vsize too.

Done too as this is safier.

Post by Kevin J. McCarthy
I think the second time is okay. The routine is just resorting, and
updating the virtual and v2r fields with the assumption that the actual
visible headers hasn't changed.

mutt_sort_headers can be called in mbox_sync_mailbox after the
mailbox has been reopened:

if (need_sort)
/* if the mailbox was reopened, the thread tree will be invalid so make
* sure to start threading from scratch. */
mutt_sort_headers (ctx, (need_sort == MUTT_REOPENED));

Before the call to mutt_sort_headers, the vsize value is incorrect
(obsolete). After the call, if the sort method is not by thread,
then it is still incorrect. The new value will be computed later,
but I think that vcount and vsize should be set to 0 as soon as
the old values become obsolete. But this would be at another place
in the code.

While testing, I've noticed an unrelated bug:

---Mutt: =test2 [Msg:46/46 Inc:10 202K/203K]---(date)---------------------(end)-

All messages are visible in the limited view, but vsize is different
from the full size. This is not consistent. This happens when starting
Mutt on this mailbox and selecting a limited view.

[Kevin J. McCarthy]

Post by Vincent Lefevre

Post by Kevin J. McCarthy
I think the second time is okay. The routine is just resorting, and
updating the virtual and v2r fields with the assumption that the actual
visible headers hasn't changed.

mutt_sort_headers can be called in mbox_sync_mailbox after the
if (need_sort)
/* if the mailbox was reopened, the thread tree will be invalid so make
* sure to start threading from scratch. */
mutt_sort_headers (ctx, (need_sort == MUTT_REOPENED));

Unless I'm missing something, this looks redundant - for the case where
mbox_check_mailbox() returns new mail or reopen.

OP_MAIN_SYNC_FOLDER will end up calling update_index() for either of
those cases, which will both perform the sort and update vcount/vsize.

OP_QUIT and OP_MAIN_CHANGE_FOLDER do the same thing.

But I do notice OP_MAIN_IMAP_LOGOUT_ALL neglected to, and should be fixed.

Post by Vincent Lefevre
---Mutt: =test2 [Msg:46/46 Inc:10 202K/203K]---(date)---------------------(end)-
All messages are visible in the limited view, but vsize is different
from the full size. This is not consistent. This happens when starting
Mutt on this mailbox and selecting a limited view.

A quick test didn't show me the problem, so I'll have to play around
with this.

[Vincent Lefevre]

Post by Kevin J. McCarthy

Post by Vincent Lefevre

Post by Kevin J. McCarthy
I think the second time is okay. The routine is just resorting, and
updating the virtual and v2r fields with the assumption that the actual
visible headers hasn't changed.

mutt_sort_headers can be called in mbox_sync_mailbox after the
if (need_sort)
/* if the mailbox was reopened, the thread tree will be invalid so make
* sure to start threading from scratch. */
mutt_sort_headers (ctx, (need_sort == MUTT_REOPENED));

Unless I'm missing something, this looks redundant - for the case where
mbox_check_mailbox() returns new mail or reopen.

Yes, according to debug messages, it seems that there are redundant
mutt_sort_headers calls.

Post by Kevin J. McCarthy
OP_MAIN_SYNC_FOLDER will end up calling update_index() for either of
those cases, which will both perform the sort and update vcount/vsize.
OP_QUIT and OP_MAIN_CHANGE_FOLDER do the same thing.
But I do notice OP_MAIN_IMAP_LOGOUT_ALL neglected to, and should be fixed.

I don't use IMAP, so that I've never done any check with it.

Post by Kevin J. McCarthy

Post by Vincent Lefevre
---Mutt: =test2 [Msg:46/46 Inc:10 202K/203K]---(date)---------------------(end)-
All messages are visible in the limited view, but vsize is different
from the full size. This is not consistent. This happens when starting
Mutt on this mailbox and selecting a limited view.

A quick test didn't show me the problem, so I'll have to play around
with this.

This occurs when the size of the mbox file is a bit less than a
multiple of 1024. I assume that the bug is present for any size,
but due to rounding, one needs some specific values modulo 1024.
This occurs also with only 1 message; see attached mbox file,
whose size if 973 bytes. There is a 1-byte discrepancy (perhaps
1 per message in the more general case).

In mx.c, vsize is computed as follows:

ctx->vsize += this_body->length + this_body->offset -
this_body->hdr_offset;

Ditto in pattern.c:

Context->vsize += this_body->length + this_body->offset -
this_body->hdr_offset;

But perhaps the issue comes from the fact that after each message
in an mbox file there must be a newline and it is not included in
length.

This problem doesn't occur with maildir.