naviserver-devel

[naviserver-devel] Ns_ConnWriteData needs Ns_ConnFlushData ?

[Vasiljevic Z. <zv...@ar...>]

Hi!

Quite often I see:

    Ns_ConnWriteData(conn, NULL, 0, 0);

This is OK but not very readable. Can we add something like

    Ns_ConnFlushData(conn, flags)

convenience wrapper that would supply the NULL buffer and
zero bytes to the Ns_ConnWriteData() ?

This would add zero functionality but would make the
programmers intention more clear.

Cheers,
Zoran

Re: [naviserver-devel] Ns_ConnWriteData needs Ns_ConnFlushData ?

[Stephen D. <sd...@gm...>]

On Tue, May 4, 2010 at 4:58 PM, Vasiljevic Zoran <zv...@ar...> wrote:
> Hi!
>
> Quite often I see:
>
>    Ns_ConnWriteData(conn, NULL, 0, 0);
>
> This is OK but not very readable. Can we add something like
>
>    Ns_ConnFlushData(conn, flags)
>
> convenience wrapper that would supply the NULL buffer and
> zero bytes to the Ns_ConnWriteData() ?
>
> This would add zero functionality but would make the
> programmers intention more clear.


Hmm... well it's not *too* often that you see it. Some of the places
where it's currently done is probably a mistake, and sometimes it's
because the api doesn't quite let you do what you need.

For example, return.c:ReturnRange() flushes the headers by calling
Ns_ConnWriteData with a null buffer before calling
Ns_ConnSendFileVec() because the latter is low level enough, for
flexibility, that it completely ignores headers, for alternate
protocols etc. But the vector sending code can actually handle memory
buffers as well as files, so ideally you'd want to dump the headers to
a string buffer then pass them on to SendVec. Or something...

What is being buffered that you're trying to flush?

Re: [naviserver-devel] Ns_ConnWriteData needs Ns_ConnFlushData ?

[Vasiljevic Z. <zv...@ar...>]

On 04.05.2010, at 18:58, Stephen Deasey wrote:

>
> What is being buffered that you're trying to flush?

The headers! It is just a kind-of 304-type of response or
a response containing just the headers, no data.

In the "previous life" I used SetRequiredHeaders
and then FlushHeaders. Now I need to set each thing
per line and then call Ns_ConnWrite with NULL buffer:

was:

          Ns_ConnSetRequiredHeaders(conn, mime, size);
          result = Ns_ConnFlushHeaders(conn, 200);

is:

          Ns_ConnSetTypeHeader(conn, mime);
          Ns_ConnSetLengthHeader(conn, size);
          Ns_ConnSetResponseStatus(conn, 200);
          if (Ns_ConnWriteData(conn, NULL, 0, 0) != NS_OK) {
              result = 0;
          } else {
              result = size;
          }

So the 2 liner becomes 8 liner. It is not that I really
care about that, but what I find obscure is the ConnWriteData.
Something like this would be more "readable"

          Ns_ConnSetTypeHeader(conn, mime);
          Ns_ConnSetLengthHeader(conn, size);
          Ns_ConnSetResponseStatus(conn, 200);
	 result = Ns_ConnFlush(conn);

So the hypothetical Ns_ConnFlush would write all that it can
and return the number of bytes written. Or something like that.
Is there some other means of achieving the above that I am not
aware of?

Cheers
Zoran

Re: [naviserver-devel] Ns_ConnWriteData needs Ns_ConnFlushData ?

[Stephen D. <sd...@gm...>]

On Wed, May 5, 2010 at 8:46 AM, Vasiljevic Zoran <zv...@ar...> wrote:
>
> On 04.05.2010, at 18:58, Stephen Deasey wrote:
>
>>
>> What is being buffered that you're trying to flush?
>
> The headers! It is just a kind-of 304-type of response or
> a response containing just the headers, no data.
>
> In the "previous life" I used SetRequiredHeaders
> and then FlushHeaders. Now I need to set each thing
> per line and then call Ns_ConnWrite with NULL buffer:
>
> was:
>
>          Ns_ConnSetRequiredHeaders(conn, mime, size);
>          result = Ns_ConnFlushHeaders(conn, 200);
>
> is:
>
>          Ns_ConnSetTypeHeader(conn, mime);
>          Ns_ConnSetLengthHeader(conn, size);
>          Ns_ConnSetResponseStatus(conn, 200);
>          if (Ns_ConnWriteData(conn, NULL, 0, 0) != NS_OK) {
>              result = 0;
>          } else {
>              result = size;
>          }
>
> So the 2 liner becomes 8 liner. It is not that I really
> care about that, but what I find obscure is the ConnWriteData.
> Something like this would be more "readable"
>
>          Ns_ConnSetTypeHeader(conn, mime);
>          Ns_ConnSetLengthHeader(conn, size);
>          Ns_ConnSetResponseStatus(conn, 200);
>         result = Ns_ConnFlush(conn);
>
> So the hypothetical Ns_ConnFlush would write all that it can
> and return the number of bytes written. Or something like that.
> Is there some other means of achieving the above that I am not
> aware of?


In general:

    result = Ns_ConnReturnData(conn, status, data, data_length, mime_type);

For responses without a body:

    result = Ns_ConnReturnStatus(conn, status);

For 304's in particular:

    result = Ns_ConnReturnNotModified(conn);

...and related in nsd/returnresp.c.  If there's a useful HTTP response
that's missing, you should probably just add it.


This example is kid of buggy:

>          Ns_ConnSetTypeHeader(conn, mime);
>          Ns_ConnSetLengthHeader(conn, size);
>          Ns_ConnSetResponseStatus(conn, 200);
>          if (Ns_ConnWriteData(conn, NULL, 0, 0) != NS_OK) {
>              result = 0;
>          } else {
>              result = size;
>          }

You've converted the NS_OK/NS_ERROR response from Ns_ConnWriteData to
a byte count, but that doesn't really tell you anything useful. In the
NS_ERROR case, some bytes might actually have been written, but you've
bashed it to zero. Setting result to size (the length header) is
misleading because the number of bytes written includes the bytes of
the headers, which the length header does not include. The byte count
can also be inflated because of character encoding and
chunked-encoding framing. It might actually be decreased because
compression.

What you can't do is compare bytes written to your original buffer
size and determine whether your write was successful. Which is one of
the reasons why FlushHeaders is depreciated.

Re: [naviserver-devel] Ns_ConnWriteData needs Ns_ConnFlushData ?

[Vasiljevic Z. <zv...@ar...>]

On 05.05.2010, at 16:54, Stephen Deasey wrote:

> In general:
>
>    result = Ns_ConnReturnData(conn, status, data, data_length,  
> mime_type);
>
> For responses without a body:
>
>    result = Ns_ConnReturnStatus(conn, status);
>
> For 304's in particular:
>
>    result = Ns_ConnReturnNotModified(conn);
>
> ...and related in nsd/returnresp.c.  If there's a useful HTTP response
> that's missing, you should probably just add it.

Perfect. Even better than I thought :-)

>
>
> This example is kid of buggy:

It is. Just made quick one to illustrate what's up.
But a good explanation! Thanks. Have learned something new.

Cheers
Zoran