On Thu, 2010-01-14 at 13:31 +0000, Christine Caulfield wrote:
> On 13/01/10 20:32, Ryan O'Hara wrote:
> > On Wed, Jan 13, 2010 at 09:58:29AM +0000, Christine Caulfield wrote:
> >> If a callback is set to NULL and a message is received for that
> >> callback, libSaLck will get into a tight loop because the continue
> >> doesn't call coroipcc_dispatch_put.
> >>
> >> This patch fixes
> >>
> >> Chrissie
> >
> >
> > I don't think the first 'goto dispatch_continue' is necessary (in the
> > case where coroipcc_dispatch_get returns TRY_AGAIN.
>
> Actually I think that probably is needed. Otherwise there is a
> coroipcc_dispatch_get() without a corresponding put. Or perhaps it ought
> to jump to error_put: ?
>
The patch looks good minus the part Ryan mentioned and is a great catch.
We need this for all the other library services as well.
Ryan is correct about the incorrect use of coroipcc_dispatch_get calling
put on TRY_AGAIN. coroipcc_dispatch_put should only be called if
dispatch_get returns CS_OK. In coroipcc.c, when TRY_AGAIN is returned
by coroipcc_dispatch_get, the original state is kept on the ipc
connection before returning. (ie it turns into a NOOP only returning
TRY_AGAIN).
Regards
-steve
>
> Chrissie
>
> > Other than that, ACK.
> >
> > Ryan
> >
> >
> >
> >> Index: lib/lck.c
> >> ===================================================================
> >> --- lib/lck.c (revision 2097)
> >> +++ lib/lck.c (working copy)
> >> @@ -332,7 +332,7 @@
> >> if (dispatchFlags == CPG_DISPATCH_ALL) {
> >> break;
> >> } else {
> >> - continue;
> >> + goto dispatch_continue;
> >> }
> >> }
> >> if (error != CS_OK) {
> >> @@ -345,7 +345,7 @@
> >> switch (dispatch_data->id) {
> >> case MESSAGE_RES_LCK_RESOURCEOPEN_CALLBACK:
> >> if (callbacks.saLckResourceOpenCallback == NULL) {
> >> - continue;
> >> + goto dispatch_continue;
> >> }
> >> res_lib_lck_resourceopen_callback =
> >> (struct res_lib_lck_resourceopen_callback
> >> *)dispatch_data;
> >> @@ -374,7 +374,7 @@
> >>
> >> case MESSAGE_RES_LCK_LOCKGRANT_CALLBACK:
> >> if (callbacks.saLckLockGrantCallback == NULL) {
> >> - continue;
> >> + goto dispatch_continue;
> >> }
> >> res_lib_lck_lockgrant_callback =
> >> (struct res_lib_lck_lockgrant_callback
> >> *)dispatch_data;
> >> @@ -421,7 +421,7 @@
> >>
> >> case MESSAGE_RES_LCK_LOCKWAITER_CALLBACK:
> >> if (callbacks.saLckLockWaiterCallback == NULL) {
> >> - continue;
> >> + goto dispatch_continue;
> >> }
> >> res_lib_lck_lockwaiter_callback =
> >> (struct res_lib_lck_lockwaiter_callback
> >> *)dispatch_data;
> >> @@ -451,7 +451,7 @@
> >>
> >> case MESSAGE_RES_LCK_RESOURCEUNLOCK_CALLBACK:
> >> if (callbacks.saLckResourceUnlockCallback == NULL) {
> >> - continue;
> >> + goto dispatch_continue;
> >> }
> >> res_lib_lck_resourceunlock_callback =
> >> (struct res_lib_lck_resourceunlock_callback
> >> *)dispatch_data;
> >> @@ -498,6 +498,7 @@
> >> default:
> >> break;
> >> }
> >> + dispatch_continue:
> >> coroipcc_dispatch_put (lckInstance->ipc_handle);
> >>
> >> switch (dispatchFlags) {
> >
> >> _______________________________________________
> >> Openais mailing list
> >> [email protected]
> >> https://lists.linux-foundation.org/mailman/listinfo/openais
>
> _______________________________________________
> Openais mailing list
> [email protected]
> https://lists.linux-foundation.org/mailman/listinfo/openais
_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais
