[Libwebsockets] RFC: more public api normalization before 1.6

Andy Green andy at warmcat.com
Thu Dec 17 11:27:56 CET 2015



On 12/17/2015 06:14 PM, Roger Light wrote:
> 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?

Yeah.  It sounds bad to me just reading about it, and it was my idea.

I pushed a patch mass-changing it over.

-Andy

> 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