Re: haproxy: new bugreports & patches & ideas

On Sun, Sep 23, 2007 at 11:57:27AM +0200, Krzysztof Oledzki wrote:
>
> On Sun, 23 Sep 2007, Willy Tarreau wrote:
> >OK so that confirms that it was indeed the problem. However, I don't like
> >your patch because it does malloc() inside I/O routines. What I suggest is
> >even simpler : you keep all the recv() and error check path intact, then
> >you add a recv() just before out_wakeup, where you inserted a free() :
> >
> > recv(fd, trash, sizeof(trash), 0);
> >
> >That will read up to BUFSIZE bytes, ignore the result (since we're not
> >interested int it) and nothing needs to be allocated/freed because the
> >"trash" buffer is designed exactly for this purpose (declared in haproxy.c
> >and include/common/globals.h).
> >
> >Test it, it should work with only one line changed.
>
> Right, I did not know about this useful trash variable. How about this
> one? I simply removed the 'reply' variable and changed it into 'trash'.

OK, that seems fine.

> I believe it is safe to use it that way - this is a single threaded
> appliaction and the variable is already used in similar way in other
> places.

Yes. The general idea is that is must be used only if you *know* that nobody else may destroy it. So even within a function, you should be careful about what the functions you use do. Basically, it's good for sprintf and things like this. Initially I used it as a write-only buffer, but sometimes it helps being able to conserve a few bytes/ kilobytes along 10 lines of code without allocating anything.

> So, it works as expected with small bonus - less stack usage. ;)

Yes, that's fine.

I'm queueing it for 1.3.12.X, thanks Krzysztof!

Willy Received on 2007/09/23 12:26