[Libwebsockets] RFC: more public api normalization before 1.6

Roger Light roger at atchoo.org
Thu Dec 17 11:14:46 CET 2015


Hi,

I've just done a quick port of my code and am happy, with one small
exception - I got stuck trying trying to find lws_get_ctx(). I read
your email late last night and forget about that function this
morning.

lws_get_ctx() is the only place in the code where a struct lws_context
is referred to as "ctx" instead of "context". Could this please be
changed to lws_get_context() for consistency, readability and
discoverability?

Cheers,

Roger


On Thu, Dec 17, 2015 at 10:06 AM, Andy Green <andy at warmcat.com> wrote:
>
>
> On 12/17/2015 08:26 AM, Andy Green wrote:
>>
>>
>>
>> On 12/16/2015 07:03 PM, Andy Green wrote:
>>>
>>>
>>>
>>> On 12/16/2015 12:05 AM, Andrejs Hanins wrote:
>>>>
>>>> Andy,
>>>>
>>>> On 12/15/2015 05:42 PM, Andy Green wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 12/15/2015 11:35 PM, Andrejs Hanins wrote:
>>>>>>
>>>>>> Hi Andy
>>>>>>
>>>>>> On 12/15/2015 03:44 PM, Andy Green wrote:
>>>>>>>
>>>>>>> Hi -
>>>>>>>
>>>>>>> I just pushed a patch that removed many internal api context
>>>>>>> parameter passings, since we gave every wsi a context pointer now.
>>>>>>> It doesn't affect anything since everything internal to the library.
>>>>>>>
>>>>>>>
>>>>>>> https://github.com/warmcat/libwebsockets/commit/6b5de70f4fb1eadac6730f3b4ecfe156bd38567a
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> However there are 11 public APIs that could get the same
>>>>>>> treatment.  I'm not sure if the context parameter to the user
>>>>>>> callback could also go, since maybe sometimes wsi is NULL there,
>>>>>>
>>>>>> Well, it depends on whether someone is interested in context pointer
>>>>>> when wsi is NULL... Can you name situations when wsi can be NULL in
>>>>>> user callbacks?
>>>>>
>>>>>
>>>>> LWS_CALLBACK_OPENSSL_PERFORM_CLIENT_CERT_VERIFICATION does it, he's
>>>>> called himself from an ssl callback with no obvious access to a wsi.
>>>>> He actually has a NULL context for the same reason.
>>>>
>>>> Well, this is strange. According to code in OpenSSL_verify_callback()
>>>> lws_context* is available there. Why it is not passed to the user
>>>> callback?
>>>
>>>
>>> OK.  But I was really checking to see if there was some big problem
>>> doing more corrective surgery on the 11 apis.  Since it seems the
>>> general feeling is get everything cleaned out now in one hit, I pushed a
>>> patch removing context parameter on those, and documenting it in
>>> changelog.
>>>
>>>> In situations when more than once context is opened it might be very
>>>> useful to know which context the SSL connection belongs to, especially
>>>> when doing some cert analysis.
>>>> User might have SSL opened on multiple ports...
>>>
>>>
>>> Yeah.  Later or tomorrow I will study what would happen in the example
>>> user code if context went away from the callback now.  There are some
>>> (3) APIs that got context added to them as a parameter because it went
>>> away from protocols.  But there is now a public api lws_get_ctx(wsi).
>>> And almost everywhere else, context is no longer needed as a parameter.
>>>
>>> I'll also look at any NULL callback params at the callback instances and
>>> see if something useful could better go there.
>>
>>
>> I pushed a patch
>>
>>
>> http://git.libwebsockets.org/cgi-bin/cgit/libwebsockets/commit/?id=d0376f8c9573cdf8ccb530dd04296b6b16f76801
>>
>>
>> that removes the context parameter from the user callback, and audits
>> every user callback action in the library to provide a valid wsi with at
>> least a valid context pointer that lws_get_ctx(wsi) can use.
>>
>> In a very few cases there's no meaningful wsi available because of when
>> or where the callback is happening, in those cases I added a temp wsi
>> that is zeroed down and has only the context pointer set.  So you never
>> get a NULL wsi coming and can always get the context from it if needed.
>>
>> Basically because of the other changes removing context when it can get
>> got from a wsi in other apis, there's hardly any use of in the the user
>> callback any more, anyway.
>>
>> The test apps are modified accordingly and changelog updated.
>
>
> There's some more cleaning done and thanks to one emptyVoid @github, the
> windows stuff is known to be in good shape both on whatever he's using and
> Appveyor (Travis for Linux / OSX is also passing everything currently).
>
> So my plan if no difficult problems, or more things that should be solved
> under the cover of the soname bump coming in the next 12h or so is to go
> with
>
> 54806b15417ff626ec8504c0666e4bf634846752
>
> +/- a patch to bump the version / changelog tomorrow morning my time.
>
> -Andy
>
>
>> -Andy
>>
>>> -Andy
>>>
>>>>> LWS_CALLBACK_PROTOCOL_INIT is called before any wsi exist.
>>>>>
>>>>> LWS_CALLBACK_GET_THREAD_ID does it because he doesn't care, but he
>>>>> does have a wsi he could pass in.
>>>>>
>>>>> -Andy
>>>>>
>>>>>>> but possibly that too.
>>>>>>>
>>>>>>> Basically all these guys can lose the context parameter now:
>>>>>>>
>>>>>>> LWS_VISIBLE LWS_EXTERN int
>>>>>>> lws_add_http_header_by_name(struct lws_context *context,
>>>>>>>                   struct lws *wsi,
>>>>>>>                   const unsigned char *name,
>>>>>>>                   const unsigned char *value,
>>>>>>>                   int length,
>>>>>>>                   unsigned char **p,
>>>>>>>                   unsigned char *end);
>>>>>>> LWS_VISIBLE LWS_EXTERN int
>>>>>>> lws_finalize_http_header(struct lws_context *context,
>>>>>>>                struct lws *wsi,
>>>>>>>                unsigned char **p,
>>>>>>>                unsigned char *end);
>>>>>>> LWS_VISIBLE LWS_EXTERN int
>>>>>>> lws_add_http_header_by_token(struct lws_context *context,
>>>>>>>                    struct lws *wsi,
>>>>>>>                    enum lws_token_indexes token,
>>>>>>>                    const unsigned char *value,
>>>>>>>                    int length,
>>>>>>>                    unsigned char **p,
>>>>>>>                    unsigned char *end);
>>>>>>> LWS_VISIBLE LWS_EXTERN int
>>>>>>> lws_add_http_header_content_length(struct lws_context *context,
>>>>>>>                      struct lws *wsi,
>>>>>>>                      unsigned long content_length,
>>>>>>>                      unsigned char **p,
>>>>>>>                      unsigned char *end);
>>>>>>> LWS_VISIBLE LWS_EXTERN int
>>>>>>> lws_add_http_header_status(struct lws_context *context, struct lws
>>>>>>> *wsi,
>>>>>>>                  unsigned int code, unsigned char **p,
>>>>>>>                  unsigned char *end);
>>>>>>>
>>>>>>> LWS_VISIBLE LWS_EXTERN int
>>>>>>> lws_serve_http_file(struct lws_context *context, struct lws *wsi,
>>>>>>>               const char *file, const char *content_type,
>>>>>>>               const char *other_headers, int other_headers_len);
>>>>>>> LWS_VISIBLE LWS_EXTERN int
>>>>>>> lws_serve_http_file_fragment(struct lws_context *context, struct
>>>>>>> lws *wsi);
>>>>>>>
>>>>>>> LWS_VISIBLE LWS_EXTERN int
>>>>>>> lws_return_http_status(struct lws_context *context, struct lws *wsi,
>>>>>>>                  unsigned int code, const char *html_body);
>>>>>>>
>>>>>>> LWS_VISIBLE LWS_EXTERN int
>>>>>>> lws_callback_on_writable(const struct lws_context *context, struct
>>>>>>> lws *wsi);
>>>>>>>
>>>>>>> LWS_VISIBLE LWS_EXTERN void
>>>>>>> lws_get_peer_addresses(struct lws_context *context, struct lws *wsi,
>>>>>>>                  lws_sockfd_type fd, char *name, int name_len,
>>>>>>>                  char *rip, int rip_len);
>>>>>>>
>>>>>>> LWS_VISIBLE LWS_EXTERN int
>>>>>>> lws_read(struct lws_context *context, struct lws *wsi,
>>>>>>>        unsigned char *buf, size_t len);
>>>>>>>
>>>>>>>
>>>>>>> Since this is fairly major, I'm checking for any outpouring of rage
>>>>>>> and grief first... but if we're going to do it now is the time I
>>>>>>> think...
>>>>>>>
>>>>>>> -Andy
>>>>>>> _______________________________________________
>>>>>>> Libwebsockets mailing list
>>>>>>> Libwebsockets at ml.libwebsockets.org
>>>>>>> http://ml.libwebsockets.org/mailman/listinfo/libwebsockets
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Libwebsockets mailing list
>>>>>> Libwebsockets at ml.libwebsockets.org
>>>>>> http://ml.libwebsockets.org/mailman/listinfo/libwebsockets
>>>>>>
>>>>
>>> _______________________________________________
>>> Libwebsockets mailing list
>>> Libwebsockets at ml.libwebsockets.org
>>> http://ml.libwebsockets.org/mailman/listinfo/libwebsockets
>>
>> _______________________________________________
>> Libwebsockets mailing list
>> Libwebsockets at ml.libwebsockets.org
>> http://ml.libwebsockets.org/mailman/listinfo/libwebsockets
>
> _______________________________________________
> Libwebsockets mailing list
> Libwebsockets at ml.libwebsockets.org
> http://ml.libwebsockets.org/mailman/listinfo/libwebsockets



More information about the Libwebsockets mailing list