master process handling patch

[Henrique de Moraes Holschuh]

On Fri, 09 Jul 2010, Patrick Goetz wrote:
> 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)

Both work, but you should try to make things easier for the compiler when
you can if it won't make the code any uglier or harder to maintain.

Trusting gcc too much to not do dumb [but not strictly incorrect] things on
random archs is never wise.

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

Yeah.  It is just that since we are going to operate on a signed variable,
it is best to keep them all signed.

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

https://alioth.debian.org/mail/?group_id=30579

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

It wasn't.  I collected the information and sent it in an email to the
debian cyrus ML, will commit to SVN now that I have internet access again.

> 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?  <:)

Heh, you can already use whatever we have in the experimental branch, it is
good enough for limited use :)

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

It is initializing crap that was not being reset when master was reloading
cyrus.conf and updating its internal service database.

Master's babysit capabilities are undocumented, too.  That also needs a
patch.

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

Yes.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh