master process handling patch

[Patrick Goetz]

On 07/02/2010 10:11 PM, Henrique de Moraes Holschuh wrote:
>>    +int read_msg(int fd, struct notify_message *msg)
>>    +{
>>    +    ssize_t r;
>>    +    size_t off = 0;
>>    +    size_t s = sizeof(struct notify_message);
>>
>>
>
> No. As I said in the debian cyrus devel list, either ssize_t s =, or
> leave it as int.  No reason to have signed/unsigned math for something
> like that.

OK, this is something of a gray area, as far as I can tell, but since 
the theoretical underflow here:
+        s -= r;
can't happen because of the context, ssize_t works
(otherwise max size_t = 2 * max ssize_t)

I'm not an expert, but ssize_t is usually used in the context where the 
return value might be negative due to an error condition; e.g.
   ssize_t read(int fd, void *buf, size_t len)
Several hours of researching this in the glibc manual, etc.. didn't 
offer any further elucidation.  People seem to use it like this: "use 
ssize_t instead of size_t when you need to allow for negative values". 
The underflow problem mentioned above is known and unresolved, AFAIK.

Also, how do I get on the Debian cyrus devel list?

> Also, we have metadata for why this thing exists, it was added by me to
> Debian Cyrus 2.1 because of problems seen in the field.  We have the
> problem documented, and the name of the reporter and tester, changelog
> entries of that time, etc.  I didn't have time to send it in yet.
>

Where is this documented?


 > Comments on the patch are welcome, but please don't merge it as is.  I
 > will clean it up a bit and add the relevant metadata on why it exists
 > and what it attempts to do.
 >

Thanks, and no problem!  I've looked at the code in 
13-master_process_handling.dpatch and decided it would take me too long 
to figure out what it's supposed to be doing.  My time right now is 
probably better spent going over the Redhat cyrus 2.3.16 rpm patches to 
see if there is anything relevant in there that needs to also be 
addressed in the Debian package.  Hmmm, if this drags on too much longer 
maybe we could just create the next debian version of cyrus like this?  <:)

    alien cyrus-imapd-2.3.16-3.fc13.i686.rpm

They even include a nice howto on reconstructing mailboxes.  <:)

While you're working on 13-master_process_handling.dpatch, this line 
looks suspicious to me:
+  Services[i].babysit = 0;
since I don't see any context for setting this equal to 0 (but again, 
I'm not following what this code does, either.

Re: below:  presumably you'll do this when you clean up these patches?


 >> diff -urNad git~/master/master.h git/master/master.h
 >> --- git~/master/master.h	2010-01-16 19:21:09.000000000 -0200
 >> +++ git/master/master.h	2010-01-16 19:28:14.289091714 -0200
 >> @@ -45,6 +45,7 @@
 >>   extern struct service *Services;
 >>   extern int allocservices;
 >>   extern int nservices;
 >> +void sighandler_setup(void);
 >>
 >>   /*
 >>    * Description of multiple address family support from
 >
 > This hunk can be dropped.
 >