Subject: expunging minds want to know

From: Adam Treister <[email-masked]>
Date: Mon, 11 Jul 94 17:52:30 -0700

I think I've got expunging working okay, but wanted to confirm an
assumption:  is the message number in the MESSAGECACHE of an expunged
message always going to be zero?

Silly me, I spent a day crashing cuz I would respond to a

mm_expunged(43) 

by looking for the message with msgNo == 43.  That would cause me to remove
message 44, and leave 43 in place til the next redraw when I'd end up in
mm_fatal().

When I started looking for msgNo == 0, things started to work.

Does this also mean that I can ignore all mm_expunged other than setting a
flag that some expunges have occurred, and then remove ALL msgs with msgNo
== 0 when the solicited reply comes back (or next time I get back to the
main event loop) ?

How long am I guarenteed that the MESSAGECACHE* will be valid?  Because of
all the broadcasting I have to do, it feels quite inefficient.  I guess in
reality it happens relatively rarely and search a bunch of linked lists a
few extra times ain't gonna kill nobody.

BTW:  Mailstrom v2a15 is on lindy.stanford.edu.  Pending correction to
assumptions explained above, this version does handle expunges,  (and does
so as they come in).

A couple people have reported pbs as the prefs are created.   I can't
reproduce it, but have put a prefs file up too, in case you have this
trouble.

But I'd appreciate any feedback from folk who have a little time to kill.

Adam

---

From: Mark Crispin <[email-masked]>
Date: Mon, 11 Jul 1994 23:40:11 -0700 (PDT)

Adam -

It's probably better to ask me these kinds of directly rather than sending it
to the entire c-client list, unless it's of general interest.

> message always going to be zero?

There are no guarantees that this will be the case, now or in the future.  In
fact, it may break in the current code, in completely unexpected and bizarre
ways.

> == 0 when the solicited reply comes back (or next time I get back to the

When you get an mm_expunged() event, the associated elt (a MESSAGECACHE*) has
already been freed (lock count decremented, and fs_give() called if the lock
count reaches zero).  In other words, it is quite likely that you are
referencing a pointer to freed memory in the algorithm you are presently
describing.  This is double-plus ungood, and will cause totally wierd bugs.

The sole purpose of the elt->msgno being set to zero is in the case where the
lock count is non-zero; that is, some resource has a pointer to the message.
The zero elt->msgno lets that resource know that there the associated message
no longer exists, and thus mail_fetchstructure(), mail_fetchtext(), etc. can
no longer be called.  Please do not try to use this as a flag.

The correct way to respond to mm_expunged(43) is to removed the 43rd element
in your per-message structure.  You should not in any way depend upon the elt.

> How long am I guarenteed that the MESSAGECACHE* will be valid?

It isn't valid, unless it's been locked; and if you use elt locking then you
are required to increment the lock count each time you copy the pointer and
call mail_free_elt() each time you drop the pointer.  Otherwise you will have
the Memory Leak From Hell.  I strongly recommend against using elt locking for
the purpose of protecting your current algorithm that depends upon elt->msgno.

-- Mark --

---

From: Adam Treister <[email-masked]>
Date: Fri, 15 Jul 94 16:50:51 -0700

Mark,

>   It's probably better to ask me these kinds of directly
>   rather than sending it to the entire c-client list,
>   unless it's of general interest.

I think this may be of general interest.  At least I'd like to hear arguments from other client
writers, cuz they may offer me better solutions that what you are proposing.

>   You should not in any way depend upon the elt.
>   The correct way to respond to mm_expunged(43) is to
>   removed the 43rd element in your per-message structure.

This is not really satisfactory to me.  

My "per-message structure" can be sorted or filtered (a subset). Msg 43 may very well not be in this
structure, or may be the 12th item.  In all other cases I have been effectively living without
knowing what message 43 is, cuz I have a pointer to the right MESSAGECACHE and can ask it for its
msgNo.

I thought I could build a temp array as the Expunge command is sent that maintains the mapping for
the life of the expunge.  But unsolicited expunges necessitate me maintaining this array in my
mailbox object ALL THE TIME, which I don't really want to do.

I hope youre sitting, cuz the best solution I can see is to change YOUR code to:

void mail_expunged (MAILSTREAM *stream,long msgno)
{
  long i;
  if (!stream->silent) mm_expunged (stream,msgno);     // **** ADAM MOVED THIS UP

  i  = msgno - 1;
  MESSAGECACHE *elt = (MESSAGECACHE *) (*mailcache) (stream,msgno,CH_ELT);
  if (elt) {			/* if an element is there */
    elt->msgno = 0;		/* invalidate its message number and free */
    (*mailcache) (stream,msgno,CH_FREE);
  }
				/* expunge the slot */
  (*mailcache) (stream,msgno,CH_EXPUNGE);
  --stream->nmsgs;		/* update stream status */
				/* notify main program of change */
//  if (!stream->silent) mm_expunged (stream,msgno);   // ****  ADAM DOES THIS FIRST
}

This propagates the mm_expunged BEFORE freeing the element so my code is guaranteed to have the
element around and lets me look for the message number, which will still be 43 in our example.
Multi-thread races could complicate this a little, but not on the Mac :)

>From what you say, I don't see how my change would break others code, if they don't rely on the
element (as they currently shouldn't), but I'd like to propose this change to future versions of
c-client, unless others shoot me down in flames.  It does seem to me its more standard programming
practice to notify dependents of changes before the notifier breaks links.

Is there a new c-client in the works, complying with IMAP4?  Assuming you're amenable to this change
(yes, its a big assumption) I have a couple other changes to request.  One is optional logging
capabilities, which I've had to insert in imap_send, imap_parse_reply, and imap_send_literal.  The
other is that I'd like to replace all the hard-coded strings with localizable string references.  I
acknowledge how evil it is to tweak underlying libraries, and would like to follow proper channels
to get all these things incorporated in the official version.

Adam

---

From: Terry Gray <[email-masked]>
Date: Fri, 15 Jul 1994 16:59:11 -0700 (PDT)

> I hope youre sitting, cuz the best solution I can see is to change YOUR 

Mark probably *is* sitting, but not in front of a terminal --he's on 
vacation til the middle of next week.

(Just didn't want you to mis-interpret any lack of response...)

-teg