[Libwebsockets] proposed change - external poll fd calls, args

"Andy Green (林安廸)" andy at warmcat.com
Sat Jan 11 06:09:35 CET 2014


On 07/01/14 20:09, the mail apparently from Michael Haberler included:
>
> Hi Andy,
>
> I'm using EXTERNAL_POLL to integrate with the zeroMQ czmq event loop
> scheme (zloop_poll(), http://czmq.zeromq.org/manual:zloop).
>
> The problem I have is:
>
> - the LWS_CALLBACK_CLEAR_MODE_POLL_FD and
> LWS_CALLBACK_SET_MODE_POLL_FD callbacks do pass the _and and _or
> masks, however they do not pass the current event mask as held in
> context->fds[wsi->position_in_fds_table].events - this forces the
> external poll support to track the current events mask even if it is
> already tracked internally (pollfds array in test-server.c) - czmq
> supports adding/deleting a poll item by fd (zloop_poller(),
> zloop_poller_end(), but not modifying an existing mask, just

Hm that sounds like a problem on zmq side, you can't even read the event 
mask let alone modify it.

> deleting/adding a poller, so I need the resulting mask, not just the
> changes
>
> The events tracking in the external loop support could be done away
> completely in this case if the _POLL_FD caOn 07/01/14 20:09, the mail apparently from Michael Haberler included:>
> Hi Andy,
>
> I'm using EXTERNAL_POLL to integrate with the zeroMQ czmq event loop scheme (zloop_poll(), http://czmq.zeromq.org/manual:zloop).
>
> The problem I have is:
>
> - the LWS_CALLBACK_CLEAR_MODE_POLL_FD and LWS_CALLBACK_SET_MODE_POLL_FD callbacks do pass the _and and _or masks, however they do not pass the current event mask as held in context->fds[wsi->position_in_fds_table].events
> - this forces the external poll support to track the current events mask even if it is already tracked internally (pollfds array in test-server.c)
> - czmq supports adding/deleting a poll item by fd (zloop_poller(), zloop_poller_end(), but not modifying an existing mask, just deleting/adding a poller, so I need the resulting mask, not just the changes
>
> The events tracking in the external loop support could be done away completely in this case if the _POLL_FD callbacks also passed in the current mask, which suggests changing how the parameters are passed (the single int 'len' makes things a bit cramped and requires the second callback).
>
> Also, I dont see a good reason to have two callbacks in this case; if the masks are passed in via a struct pointer this can be done in one go with a, say, LWS_CALLBACK_CHANGE_MODE_POLL_FD which is a superset of LWS_CALLBACK_CLEAR_MODE_POLL_FD and LWS_CALLBACK_SET_MODE_POLL_FD.
>
>
> My proposal is:
>
> - delete the LWS_CALLBACK_CLEAR_MODE_POLL_FD and LWS_CALLBACK_SET_MODE_POLL_FD callbacks.
> - add a LWS_CALLBACK_CHANGE_MODE_POLL_FD callback which entails the functionality of the previous two.
> - change the parameter passing for LWS_CALLBACK_ADD_POLL_FD, LWS_CALLBACK_DEL_POLL_FD, LWS_CALLBACK_CHANGE_MODE_POLL_FD, LWS_CALLBACK_LOCK_POLL, LWS_CALLBACK_UNLOCK_POLL as follows:
>
> use a struct pointer which is passed in the 'in' pointer and deprecate the use of 'len' for arguments
>      struct pollargs {
> 	int fd;
> 	int events; // this will expose the pre-change current mask
> 	int _and;   // alternatively, instead of _and and _or, the post-change mask could be passed in
> 	int _or;
>      };
>
> functionally this is a superset of the current callbacks. upside: easier integration with external loops without tracking the masks.

That sounds pretty reasonable.

> --
>
> Please let me know if you would consider merging such a change, in which case I would clean up and document those changes, and submit a patch. Would a czmq integration example be of interest?

Yeah it breaks the API but I think it's good to get rid of the remaining 
callback argument overloads.

If you can sort out a patch I'll certainly be interested.

ZMQ generally I personally didn't get the point of, but you're not the 
first person to ask about it, so if you separately want to contribute 
some patch that provides documentation or support I'd certainly consider 
that too.

Thanks.

-Andy

>
>
>
> thanks in advance,
>
> Michael
>
>
> _______________________________________________
> Libwebsockets mailing list
> Libwebsockets at ml.libwebsockets.org
> http://ml.libwebsockets.org/mailman/listinfo/libwebsockets
>

llbacks also passed in the
> current mask, which suggests changing how the parameters are passed
> (the single int 'len' makes things a bit cramped and requires the
> second callback).
>
> Also, I dont see a good reason to have two callbacks in this case; if
> the masks are passed in via a struct pointer this can be done in one
> go with a, say, LWS_CALLBACK_CHANGE_MODE_POLL_FD which is a superset
> of LWS_CALLBACK_CLEAR_MODE_POLL_FD and
> LWS_CALLBACK_SET_MODE_POLL_FD.
>
>
> My proposal is:
>
> - delete the LWS_CALLBACK_CLEAR_MODE_POLL_FD and
> LWS_CALLBACK_SET_MODE_POLL_FD callbacks. - add a
> LWS_CALLBACK_CHANGE_MODE_POLL_FD callback which entails the
> functionality of the previous two. - change the parameter passing for
> LWS_CALLBACK_ADD_POLL_FD, LWS_CALLBACK_DEL_POLL_FD,
> LWS_CALLBACK_CHANGE_MODE_POLL_FD, LWS_CALLBACK_LOCK_POLL,
> LWS_CALLBACK_UNLOCK_POLL as follows:
>
> use a struct pointer which is passed in the 'in' pointer and
> deprecate the use of 'len' for arguments struct pollargs { int fd;
> int events; // this will expose the pre-change current mask int _and;
> // alternatively, instead of _and and _or, the post-change mask could
> be passed in int _or; };
>
> functionally this is a superset of the current callbacks. upside:
> easier integration with external loops without tracking the masks.
>
> --
>
> Please let me know if you would consider merging such a change, in
> which case I would clean up and document those changes, and submit a
> patch. Would a czmq integration example be of interest?
>
>
>
>
> thanks in advance,
>
> Michael
>
>
> _______________________________________________ Libwebsockets mailing
> list Libwebsockets at ml.libwebsockets.org
> http://ml.libwebsockets.org/mailman/listinfo/libwebsockets
>




More information about the Libwebsockets mailing list