Re: [libmicrohttpd] suspend/resume patch

[Christian Grothoff]

Hi Flavio,

Well, if we do bother to detect this, my preference would be not to
ignore the bad behavior, but to literally call the panic function (i.e.
abort()).  Suspending an already suspended connection or resuming one
that isn't already resumed is indicative of a serious logic error in the
application.

Yes, we could return an error code, but that'll just result in lots
of applications doing (assert(OK == MHD_suspend/resume/...())) which
isn't really any better.

My 2 cents

Christian

On 04/14/2016 07:38 PM, Flavio Ceolin wrote:
> Hi folks,
> 
> Is there any reason for microhttpd do not check if a connections is
> already resumed or suspended and handle this, instead of have undefined
> behavior ? As far as I could see, the connection struct already has the
> suspended field, so it's just a simple check.
> 
> For the user perspective is quite annoying and prone to error have to
> maintain the connection status in its side. Bellow the patch, any
> suggestion/explanation is welcome.
> 
> 
> Index: src/include/microhttpd.h
> ===================================================================
> --- src/include/microhttpd.h  (revision 37040)
> +++ src/include/microhttpd.h  (working copy)
> @@ -1957,10 +1957,7 @@
>  
>  /**
>   * Resume handling of network data for suspended connection.  It is
> - * safe to resume a suspended connection at any time.  Calling this
> - * function on a connection that was not previously suspended will
> - * result in undefined behavior.
> - *
> + * safe to resume a suspended connection at any time.
>   * @param connection the connection to resume
>   */
>  _MHD_EXTERN void
> Index: src/microhttpd/daemon.c
> ===================================================================
> --- src/microhttpd/daemon.c   (revision 37040)
> +++ src/microhttpd/daemon.c   (working copy)
> @@ -1714,6 +1714,8 @@
>  {
>    struct MHD_Daemon *daemon;
>  
> +  if (connection->suspended)
> +    return;
>    daemon = connection->daemon;
>    if (MHD_USE_SUSPEND_RESUME != (daemon->options & MHD_USE_SUSPEND_RESUME))
>      MHD_PANIC ("Cannot suspend connections without enabling 
> MHD_USE_SUSPEND_RESUME!\n");
> @@ -1781,6 +1783,9 @@
>  {
>    struct MHD_Daemon *daemon;
>  
> +  if (!connection->suspended)
> +    return;
> +
>    daemon = connection->daemon;
>    if (MHD_USE_SUSPEND_RESUME != (daemon->options & MHD_USE_SUSPEND_RESUME))
>      MHD_PANIC ("Cannot resume connections without enabling 
> MHD_USE_SUSPEND_RESUME!\n");
>

signature.asc
Description: OpenPGP digital signature