Re: [bug-mailutils] Merging libraries (was Re: Mailutils 0.3 released)

[Alain Magloire]

> 
> > I would recommand to branch the cvs tree.
> 
> No problem. But we should sketch a roadmap first, I believe.
> 
> Let's return to our discussion about merging the
> libraries. My proposition was to carry it in the following way:
> 
> [points 1,2) are already done]
> > 3) Normalize the namespace
> > 4) Reorganize headers (mailutils, mailutils/sys, etc.)
> > 5) Merge mailbox2 module by module.
> 
> Then, we decided we should agree on the following questions (reordered
> by priority):
> 
> 1) the overall "framework" of the library
> 2) the overall separation of the layers
> 3) the layout of the structures
> 4) the memory management
> 5) threading issues
> 6) the non-bloking I/O issues
> 
> So, let's "begin at the beginning":
> 
> 1) the overall "framework" of the library
> 
> I guess we should try to preserve the actual framework. It has proved
> to be suitable for its purposes and has grown to be rather "stable".
> 
> Here are the things that should definitely be fixed:
> 
> a) The use of "owner" argument in object destructors must be
> standartized. Some of them use this argument (e.g. stream_destroy),
> some don't (e.g. address_destroy). If I'm not mistaken the "owner"
> argument was meant as an additional precaution against memory-freeing
> errors. But _get_owner calls allow to override this measure, for
> example calling
> 
>       stream_destroy(&str, stream_get_owner(str))
> 
> will happily destroy the 'str' no matter is it right or wrong in the
> actual context.
> 
> I propose to get rid of "owner" argument. Instead, we should provide
> a notification mechanism with which an object being destructed could
> notify its owner about the fact. E.g.:
> 
> void
> stream_destroy (stream_t *pstream)
> {
>    if (pstream && *pstream)
>     {
>       stream_t stream = *pstream;
>       notify_owner (stream->owner, EV_DESTROY, stream);
>       ...
> 

Yes, the idea of the owner was to protect agains bad free();
So the object was the _owner_ of its parts 8-).  For example
message_t was the owner of header_t, body_t, attibute_t
        mesage_get_header( .. &header); 
        message_get_body();

so when doing :
        header_destroy(&header);
it was not really destroy but only the original message could destroy it, that
allows the message to cache its header for other messge_get_header() calls.

Now, as you pointed, it was not standardize, and not all the object/typedef
in mailutils needed this notion of "ownership".

I would rather advocate for the use of a reference count, by doing:

{
  header_t header;
  message_t mesg = mailbox_get_message(mbox, 2);
  // The header ref count  is incremented;
  message_get_header(mesg, &header);
  ... 
  // header ref count is decremented.
  header_destroy(&header);

  // destroy the message and its parts i.e the ones with refcount == 0
  message_destroy(&mesg);
}

The big problem with this, if an application does not call the xxx_destroy()
The application will leak.

In brief, I want to call say:
        message_get_header()
xx number of times and would like to have this function lightweith i.e.
it does not malloc() each but caches the result.  But would also like
to notify message_t that it is no longer necessary to hold on to it and
free() the memory.  

Your proposition about notification is nice, but how does can it help
it the case describe above?

> b) Any information retrieving function should have three interfaces:
> 

300% on this, its time to be consistent.

> 1) int <object>_get_value(<object>_t obj,
>                           [object-specific arguments,]
>                           char *buf, size_t size, size_t *n)
> 
> Stores as much as SIZE-1 bytes of object's value in BUF, adds the
> terminal zero. The amount of data actually written is stored in the
> memory location pointed to by N, unless it is NULL. If BUF is NULL,
> the actual length of the object's value is stored in N.
> 

example please.


> 2) int <object>_aget_value(<object>_t obj,
>                            [object-specific arguments,]
>                            char **bufp)
> 
> Allocates a copy of the object's value and returns the pointer to the
> allocated memory in BUFP
> 

Example.

> 3) int <object>_sget_value(<object>_t obj,
>                            [object-specific arguments,]
>                            const char **bufp)
> 
> Returns the pointer to the internally allocated data for this object.
> This will be extremely useful as it will provide fast read-only access
> to the data without unnecessary memory allocation. Lots of code in
> mail,imap4d,mh etc would be simplified if we had this.
> 
> c) The header_t object should be responsible for caching the header
> values, not the underlying header implementation.
> 

Ok.
I do not remember what we do for POP3, but the only way
to do this in POP is to download the entire header with TOP.


> opinions?

I like your idea of doing this incrementally, even if will take more time.
Short calculated step is probably better then giant leap in the unknown.