Theldus / wsServer

wsServer - a tiny WebSocket server library written in C
https://theldus.github.io/wsServer
GNU General Public License v3.0
422 stars 80 forks source link

Added user defined object to onopen, onclose and onmessage #57

Closed terziev-viktor closed 4 weeks ago

terziev-viktor commented 2 years ago

I needed to associate a thread with every client in an app Im developing. Feel free to use these changes if you like them

Theldus commented 2 years ago

Hi @terziev-viktor, thanks for the PR. This PR is clearly related to the issue #48. In the issue, it is discussed whether to support a user-defined pointer and the best way to do so.

I liked the way you approached this: saving the pointer on the stack for each client/connection, rather than the client structure itself: it's simple and saves memory.

However, although cleaner, I would really like to avoid having an additional parameter in the events of on_[open,close,message], and I would like there to be a method to possibly save and retrieve this data, from the client.

As illustrated in the issue, something like:

void ws_set_client_context(ws_cli_conn_t *cli, void *ptr);
void *ws_get_client_context(ws_cli_conn_t *cli);

Note 1: Although it is interesting to illustrate the feature, 'mouse_new' and 'mouse_free' are meant to be invoked only once, at the beginning and end of the program, respectively.

The 'mouse_new()' function initializes whatever is necessary for the OS to be able to programmatically perform mouse events. Conversely, 'mouse_free()' deallocates previously allocated resources.

That said, not only is it bad practice to do this on every connection, it may not work as expected.


Note 2: Your commit Fixed the ws_getaddress calls in the example code was already merged in your previous PR, you don't need to repeat it in this PR.


Note 3: Your email address differs from the one used in your GitHub account. If I accept your PR as is, your commit won't be counted in the 'contributors' list on GitHub.

If you want to hide it for some reason, no problem, but if you want, rebase your commit by changing the author to the same used in your GitHub account.

Theldus commented 4 weeks ago

Hi @terziev-viktor, I'm sorry, but I'm closing this PR... since a similar feature was implemented in #91.