[Openais] Re: recent segfault
Steven Dake
sdake at mvista.com
Thu Feb 3 15:20:14 PST 2005
On Thu, 2005-02-03 at 15:51, Mark Haverkamp wrote:
> On Thu, 2005-02-03 at 15:12 -0700, Steven Dake wrote:
> > Mark
> >
> > Cool I'll start testing with it.
> >
> > I have noticed there is definately still some bugs; I think related to
> > recovery in totemsrp. I've got a handle on these cases, but I'm not
> > quite sure how to fix them yet. I did notice a possible bug in your
> > patch when I looked at it earlier.
> >
> > In the previous patch you sent I noticed a problem and fixed it. I
> > think this code has the same problem. Here is the code:
> >
> > for (i = 0; i < msg_count; i++) {
> > /*
> > * If the first packed message is a continuation
> > * of a previous message, but the assembly buffer
> > * is empty, then we need to discard it since we can't
> > * assemble a complete message.
> > */
> > if (mcast->continuation && (assembly->index == 0)) {
> > assembly->index += msg_lens[i];
> > mcast->continuation = 0;
> > continue;
> > }
> >
> > assembly->index += msg_lens[i];
> > app_deliver_fn (source_addr, &iov_delv, 1,
> > endian_conversion_required);
> > iov_delv.iov_base = &assembly->data[assembly->index];
> > iov_delv.iov_len = msg_lens[i + 1];
> > }
> >
> > in the case of continuation being set, assembly->index is increased.
> > Shouldn't iov_delv.iov_base and iov_delv.iov_len should also be moved to
> > the next message?
> >
>
> > Something like:
> >
> > if (mcast->continuation && (assembly->index == 0)) {
> > assembly->index += msg_lens[0];
> > mcast->continuation = 0;
> > iov_delv.iov_base = &assembly->data[msg_lens[0]];
> > iov_delv.iov_len = msg_lens[1];
> > continue;
> > }
>
> Yes, I missed that.
>
> >
> > Also it isn't valid to change mcast->continuation. This value must be
> > cached if you intend to change it. The reason is that it may be
> > delivered to one processor, but then another processor requests it be
> > retransmitted. If that happened, the retransmit list would contain the
> > modified header which would .
>
> That makes sense, but I saw that you did a swab16 on the count values in
> the endian_coversion_required if statement which changed them, so I
> thought that it would be OK.
>
yup my bad. I thought I had made a copy of this data, but only made a
copy in one case. We will have to clean this issue up though.
> >
> > All data in the delivered message must be read only for this reason.
> >
More information about the Openais
mailing list