[Openais] Re: recent segfault
Mark Haverkamp
markh at osdl.org
Thu Feb 3 14:51:52 PST 2005
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.
>
> All data in the delivered message must be read only for this reason.
>
--
Mark Haverkamp <markh at osdl.org>
More information about the Openais
mailing list