[Openais] [openais] [whitetank] - Patch totemsrp so orf_token_mcast will not fall in case message_item->iov_len == IOVLEN

[Steven Dake]

good for merge

regards
-steve

On Thu, 2009-10-29 at 18:16 +0100, Jan Friesse wrote:
> Reworked patch.
> 
> Regards,
>   Honza
> 
> Steven Dake wrote:
> > On Tue, 2009-10-27 at 18:42 -0500, Ryan O'Hara wrote:
> >> On Tue, Oct 27, 2009 at 06:18:44PM +0100, Jan Friesse wrote:
> >>> See patch. I hope this will fix  
> >>> https://bugzilla.redhat.com/show_bug.cgi?id=525280.
> >>>
> >>> Regards,
> >>>   Honza
> >>>
> >> Do we really need SORT_QUEUE_ITEM_MAXIOVS? Perhaps using "MAXIOVS + 1"
> >> in the EVT code is sufficient.
> >>
> > 
> > My general response to this is that if we are running into the MAXIOVS
> > constraint on the data structure, increasing MAXIOVS by 1 is not going
> > to solve the problem (although it may hide this fault condition).
> > 
> > The use of an extra define as you point out is unnecessary.
> > 
> > Regards
> > -steve
> > 
> >> Ryan
> >>
> >>
> >>> diff --git a/branches/whitetank/exec/totemsrp.c b/branches/whitetank/exec/totemsrp.c
> >>> index c11a552..c5a74e8 100644
> >>> --- a/branches/whitetank/exec/totemsrp.c
> >>> +++ b/branches/whitetank/exec/totemsrp.c
> >>> @@ -87,6 +87,7 @@
> >>>  #define RETRANS_MESSAGE_QUEUE_SIZE_MAX		500 /* allow 500 messages to be queued */
> >>>  #define RECEIVED_MESSAGE_QUEUE_SIZE_MAX		500 /* allow 500 messages to be queued */
> >>>  #define MAXIOVS					5	
> >>> +#define SORT_QUEUE_ITEM_MAXIOVS			MAXIOVS + 1
> >>>  #define RETRANSMIT_ENTRIES_MAX			30
> >>>  #define TOKEN_SIZE_MAX				64000 /* bytes */
> >>>  
> >>> @@ -278,7 +279,7 @@ struct message_item {
> >>>  };
> >>>  
> >>>  struct sort_queue_item {
> >>> -	struct iovec iovec[MAXIOVS];
> >>> +	struct iovec iovec[SORT_QUEUE_ITEM_MAXIOVS];
> >>>  	int iov_len;
> >>>  };
> >>>  
> >>> @@ -1916,7 +1917,7 @@ static void memb_state_recovery_enter (
> >>>  	strcat (is_originated, seqno_string_hex);
> >>>  	sort_queue_item = ptr;
> >>>  	assert (sort_queue_item->iov_len > 0);
> >>> -	assert (sort_queue_item->iov_len <= MAXIOVS);
> >>> +	assert (sort_queue_item->iov_len <= SORT_QUEUE_ITEM_MAXIOVS);
> >>>  	messages_originated++;
> >>>  	memset (&message_item, 0, sizeof (struct message_item));
> >>>  // TODO	 LEAK
> >>> _______________________________________________
> >>> Openais mailing list
> >>> Openais at lists.linux-foundation.org
> >>> https://lists.linux-foundation.org/mailman/listinfo/openais
> >> _______________________________________________
> >> Openais mailing list
> >> Openais at lists.linux-foundation.org
> >> https://lists.linux-foundation.org/mailman/listinfo/openais
> > 
> 
> plain text document attachment
> (whitetank-totemsrp-fix-orf_token_mcast-take2.patch)
> diff --git a/branches/whitetank/exec/totemsrp.c b/branches/whitetank/exec/totemsrp.c
> index c11a552..1cd2099 100644
> --- a/branches/whitetank/exec/totemsrp.c
> +++ b/branches/whitetank/exec/totemsrp.c
> @@ -86,7 +86,7 @@
>  #define QUEUE_RTR_ITEMS_SIZE_MAX		256 /* allow 256 retransmit items */
>  #define RETRANS_MESSAGE_QUEUE_SIZE_MAX		500 /* allow 500 messages to be queued */
>  #define RECEIVED_MESSAGE_QUEUE_SIZE_MAX		500 /* allow 500 messages to be queued */
> -#define MAXIOVS					5	
> +#define MAXIOVS					10
>  #define RETRANSMIT_ENTRIES_MAX			30
>  #define TOKEN_SIZE_MAX				64000 /* bytes */
>  
> @@ -2030,6 +2030,8 @@ int totemsrp_mcast (
>  	message_item.mcast->guarantee = guarantee;
>  	srp_addr_copy (&message_item.mcast->system_from, &instance->my_id);
>  
> +	assert (iov_len <= MAXIOVS);
> +
>  	for (i = 0; i < iov_len; i++) {
>  // TODO LEAK
>  		message_item.iovec[i].iov_base = malloc (iovec[i].iov_len);
> @@ -2276,6 +2278,7 @@ static int orf_token_mcast (
>  		message_item->mcast->seq = ++token->seq;
>  		message_item->mcast->this_seqno = instance->global_seqno++;
>  
> +		assert (message_item->iov_len < MAXIOVS);
>  		/*
>  		 * Build IO vector
>  		 */
> @@ -2292,8 +2295,6 @@ static int orf_token_mcast (
>  
>  		sort_queue_item.iov_len = message_item->iov_len + 1;
>  
> -		assert (sort_queue_item.iov_len < 16);
> -
>  		/*
>  		 * Add message to retransmit queue
>  		 */