[Libwebsockets] RFC: more public api normalization before 1.6

Andrejs Hanins andrejs.hanins at ubnt.com
Thu Dec 17 18:34:25 CET 2015



On 12/17/2015 07:10 PM, Andy Green wrote:
>
> On December 17, 2015 11:55:34 PM GMT+08:00, Andrejs Hanins <andrejs.hanins at ubnt.com> wrote:
>> Andy,
>>
>> On 12/17/2015 12:06 PM, Andy Green 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
>> LWS_CALLBACK_PROTOCOL_DESTROY is called with wsi=NULL, looks like easy
>> to fix with temporary wsi... Again, user needs to know which context is
>> being destroyed.
> OK, thanks for spotting it.  I gave him also the dummy wsi that has context treatment.
Yeah, process crash spotted it for me :)
Otherwise, I also fine with removing context ptr from API, my code works.

>
> -Andy
>
>>> 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




More information about the Libwebsockets mailing list