Jim,
Try using this patch with your patchset.
It makes a copy of the constant void *msg into a new merge_detect
struct.
If it looks good go ahead and merge in with your changes on wednesday.
Regards
-steve
On Mon, 2009-04-13 at 17:02 +0200, Jim Meyering wrote:
> Steven Dake wrote:
> > On Fri, 2009-04-10 at 18:57 +0200, Jim Meyering wrote:
> >> Here's a big const-adding change that doesn't touch APIs much.
> >> However, it's important for internal consistency.
> >>
> >> There is one snag, though, which means this patch isn't quite
> >> committable:
> >>
> >> The first clue was a prototype mismatch for
> >> message_handler_memb_merge_detect. It seemed like
> >> it should be like the 5 other handlers here:
> >> [this is in totemsrp.c]
> >>
> >> struct message_handlers {
> >> int count;
> >> int (*handler_functions[6]) (
> >> struct totemsrp_instance *instance,
> >> const void *msg,
> >> size_t msg_len,
> >> int endian_conversion_needed);
> >> };
> >>
> >> struct message_handlers totemsrp_message_handlers = {
> >> 6,
> >> {
> >> message_handler_orf_token,
> >> message_handler_mcast,
> >> message_handler_memb_merge_detect,
> >> message_handler_memb_join,
> >> message_handler_memb_commit_token,
> >> message_handler_token_hold_cancel
> >> }
> >> };
> >>
> >> and have a "const" "msg" parameter.
> >> However, currently it calls memb_merge_detect_endian_convert (msg,
> >> msg) which modifies *msg, which implies its "msg" can't be "const".
> >> Yet nothing else in that function uses the modified "msg". Maybe the
> >> caller requires that *msg be modified?
> >>
> >
> > The msg parameter is used in the assignment to merge_detect_timeout
> > variable.
> >
> > The endian conversion is absolutely necessary.
> >
> > What I suggest is to make a second variable which is the endian
> > converted version and either a) endian convert if necessary or b) memcpy
> > the message to the data structure if not endian converting.
> > |
> > If you don't know what I mean, let me know and I'll work out a patch.
>
> Thanks.
>
> I think your reply means that once const'ified, the 6 handlers may not
> all have the same type, since the 'merge_detect'ing one modifies *msg
> and for the others, that parameter is const. In that case, the least
> invasive change is to adjust the declaration of "struct message_handlers".
> No problem.
>
> I'm out this week except for Wednesday, and will fix it one way or
> another then.
Index: exec/totemsrp.c
===================================================================
--- exec/totemsrp.c (revision 2057)
+++ exec/totemsrp.c (working copy)
@@ -3738,16 +3738,20 @@
int msg_len,
int endian_conversion_needed)
{
- struct memb_merge_detect *memb_merge_detect = (struct memb_merge_detect *)msg;
+ struct memb_merge_detect memb_merge_detect;
+
if (endian_conversion_needed) {
- memb_merge_detect_endian_convert (msg, msg);
+ memb_merge_detect_endian_convert (msg, &memb_merge_detect);
+ } else {
+ memcpy (&memb_merge_detect, msg,
+ sizeof (struct memb_merge_detect));
}
/*
* do nothing if this is a merge detect from this configuration
*/
- if (memcmp (&instance->my_ring_id, &memb_merge_detect->ring_id,
+ if (memcmp (&instance->my_ring_id, &memb_merge_detect.ring_id,
sizeof (struct memb_ring_id)) == 0) {
return (0);
@@ -3758,19 +3762,19 @@
*/
switch (instance->memb_state) {
case MEMB_STATE_OPERATIONAL:
- memb_set_merge (&memb_merge_detect->system_from, 1,
+ memb_set_merge (&memb_merge_detect.system_from, 1,
instance->my_proc_list, &instance->my_proc_list_entries);
memb_state_gather_enter (instance, 9);
break;
case MEMB_STATE_GATHER:
if (!memb_set_subset (
- &memb_merge_detect->system_from,
+ &memb_merge_detect.system_from,
1,
instance->my_proc_list,
instance->my_proc_list_entries)) {
- memb_set_merge (&memb_merge_detect->system_from, 1,
+ memb_set_merge (&memb_merge_detect.system_from, 1,
instance->my_proc_list, &instance->my_proc_list_entries);
memb_state_gather_enter (instance, 10);
return (0);
_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais
