Re: POP3d locking

[Sergey Poznyakoff]

There was an error in proposed mbox_is_updated() (c'est l'esprit
d'escalier 8-) It should read:

static int
mbox_is_updated (mailbox_t mailbox)
{
  off_t size = 0;
  mbox_data_t mud = mailbox->data;
  if (mud->size == 0 || stream_size (mailbox->stream, &size) != 0)
    return 0;
  if (size < mud->size)
    {
      observable_notify (mailbox->observable, MU_EVT_MAILBOX_CORRUPT);
      /* And be verbose.  ? */
      error("* BAD : Mailbox corrupted, shrank size");
      /* FIXME: should I crash.  */
      return 0;
    }
  return (mud->size == size);
}

Then the proposed change for pop3_mainloop() is

Index: pop3d.c
===================================================================
RCS file: /cvs/mailutils/pop3d/pop3d.c,v
retrieving revision 1.22
diff -c -r1.22 pop3d.c
***************
*** 247,253 ****
        buf = pop3_readline (ifile);
        cmd = pop3_cmd (buf);
        arg = pop3_args (buf);
! 
        if (strlen (arg) > POP_MAXCMDLEN || strlen (cmd) > POP_MAXCMDLEN)
        status = ERR_TOO_LONG;
        else if (strlen (cmd) > 4)
--- 264,273 ----
        buf = pop3_readline (ifile);
        cmd = pop3_cmd (buf);
        arg = pop3_args (buf);
!       
!       if (state == TRANSACTION && !mailbox_is_updated (mbox)) 
!       pop3_abquit(ERR_MBOX_SYNC);
!        
        if (strlen (arg) > POP_MAXCMDLEN || strlen (cmd) > POP_MAXCMDLEN)
        status = ERR_TOO_LONG;
        else if (strlen (cmd) > 4)

But for it to work we have to scan the mailbox immediately after
entering transaction state. Otherwise after receiving the very first
command in transaction state mbox_is_updated() returns 0 and pop3d
exits. How do you think, can we afford doing mailbox scan immediately
after successfull authentication? It will be scanned anyway, sooner
or later, so it doesn't seem to make a difference...

One more thing: there seems to be an inconsistency in mbox_expunge().
Namely:


Index: mailbox/mbx_mbox.c
===================================================================
RCS file: /cvs/mailutils/mailbox/mbx_mbox.c,v
retrieving revision 1.43
diff -c -r1.43 mbx_mbox.c
*** mbx_mbox.c  2001/04/17 03:31:19     1.43
--- mbx_mbox.c  2001/04/24 10:29:02
***************
*** 700,710 ****
          if (mum->message == 0)
            {
              message_t msg;
!             status = mbox_get_message (mailbox, i, &msg);
              if (status != 0)
                {
                  fprintf (stderr, "Error expunge:%d: %s", __LINE__,
                           strerror (status));
                  goto bailout0;
                }
            }
--- 703,713 ----
          if (mum->message == 0)
            {
              message_t msg;
!             status = mbox_get_message (mailbox, i+1, &msg);
              if (status != 0)
                {
                  fprintf (stderr, "Error expunge:%d: %s", __LINE__,
                           strerror (status));
                  goto bailout0;
                }
            }


The reason for this is:

Immediately after `if (mum->message == 0)' block the following is
executed:

    status = mbox_append_message0 (tmpmailbox, mum->message,
                                         &total, 1, (i == save_imapbase));

i.e. it assumes that mbox_get_message() will place the pointer to
message into mum->message member. But mbox_get_message() relies on
1-based message numbers, whereas `mum' pointer is obtained using
0-based indexing:

  /* Copy to the temporary mailbox emails not mark deleted.  */
  for (i = dirty; i < mud->messages_count; i++)
    {
      mum = mud->umessages[i];


So, the function bails out when it comes accross a message marked dirty.


-Sergey