Hi folks,
Any other thoughts about this ? Should I consider this change not wanted
?
> Hi Christian,
>
> > 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
>
> Thanks for the fast reply. I agree that is a logic error in the
> application, but I do not see this as an error (from the microhttpd
> perspective), I mean, is quite usual this kind of functions just return
> if there is nothing to do.
>
> My usage is a kind of wrapper for libmicrohttpd, in my case, the
> responses are always asynchronous. When a request comes I suspend the
> connection and give the request to an user callback, then the user call
> a send_response() wherever he wants and in this point a resume the
> connection. To do this, I need to keep the connection status in my
> handle to avoid resume more than one and also to resume before stop the
> daemon.
>
> We can live with this of course, but i think this will become the usage
> simpler and this will not be a performance problem because this is not a
> critical function, probably is called few times in an application.
>
> >
> > 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");
Regards,
Flavio Ceolin
Intel Open source Technology Center
