no reply patch

[dormando]

Tomash Brechko wrote:
> On Mon, Feb 04, 2008 at 19:14:21 -0800, Brian Aker wrote:
>> The patch looks fine, though I think you should move this:
>>
>> +    if (c->noreply) {
>> +        c->noreply = false;
>> +        conn_set_state(c, conn_read);
>> +        return;
>> +    }
>> +
>>
>> ... to being after the verbose if () since without it debugging  
>> problems is going to be a real pain.
> 
> Thanks for looking into this.  Re your comment, the piece
> 
>     if (settings.verbose > 1)
>         fprintf(stderr, ">%d %s\n", c->sfd, str);
> 
> is the debug message about what will be send to the client.  But with
> 'noreply' nothing will be send, so I avoid this message too.  Also,
> not seeing the result actually stresses the fast that with 'noreply'
> you can't be sure of the command outcome ;).
> 
> It could be implemented either way of course, but to my personal taste
> (have no technical reasons) this doesn't have to be fixed.  Thanks
> anyway.

For sanity I'm going to have to change it to look like this:

    if (c->noreply) {
        if (settings.verbose > 1)
            fprintf(stderr, ">%d NOREPLY %s\n", c->sfd, str);
        c->noreply = false;
        conn_set_state(c, conn_read);
        return;
    }

I know we can test for the absense of something, but it's going to be
too much of a pain to externally verify results without this.

...and for both of these, the test should be 'if unlikely(c->noreply)' -
branch prediction misses are right up there with "memory's hella slow"
for slowing down a modern processor.

This also needs some more careful documentation warning about the
occasional bad line potentially throwing clients out of whack. Since
this is my pet peeve I'll add it myself.

> 
>> One thought I had with this is that you could use the token you have  
>> added to the server as an additional action verb for future commands.
> 
> Indeed, the 'more' keyword I mentioned in other mail may be used in
> this place, as it is mutually exclusive with 'noreply'.

Although as Tomash points out here... you're limited to options which
are mutually exclusive to noreply.

Tired now, but I'll merge this shortly... No one else has complained so far.

-Dormando