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.
>

Reply via email to