mod_layout - what is wrong?
witekfl at gazeta.pl
Thu Jan 7 23:04:43 MSK 2010
Dnia czwartek 07 styczeń 2010 o 00:39:14 Maxim Dounin napisał(a):
> On Wed, Jan 06, 2010 at 04:09:56PM +0100, witekfl Gazeta.pl wrote:
> > Hi,
> > http://rkd.republika.pl/ngx_http_layout_filter_module.c
> > I want to write the mod_layout.
> > The idea is: write all chains to temp file, at the end read it, insert
> > the header after <body> and the footer before </body>.
> > Sometimes it fails. There is only the header and the footer. Why?
> > Could you review it?
> Some random comments in random order:
> 1. It's probably good idea to polish style a bit before posting
> code for review.
Oh sorry. nginx uses spaces, I prefer tabs.
There is mix there.
> 2. The whole idea of buffering to temporary file looks wrong.
> There is no need to buffer more than several bytes here. And the
> only case where you need buffering is when you find something like
> "</b" at the end of buffer, and need next buffer to find out if
> it's "</body>" or "</b>". It requires a bit more complicated
> matching code, but saves lots of resources. See sub filter
> and ssi filter for examples.
I don't think it is so easy. If there is no <body> the header is added at the
begining. But one doesn't know if there is no <body> unless the whole
document is loaded.
> 3. After writing chain to temp file you have to mark buffers in
> this chain as sent by updating buf->pos. This will allow upper
> layers to reuse this buffers. Note well: you probably want to
> test your code with something like
> output_buffers 1 64;
> to see what happens if upper layers are out of buffers since you
> don't mark them as sent. Hint: request will just hang awaiting
> some buffers to become free after first 64 bytes.
> 4. layout_ignore_uris setting doesn't fit into nginx model of
> doing things. One should write correct configuration using
> "location" directives instead of doing some fancy filtering at
> module level.
It is a shared hosting. nginx.conf is complicated already.
> 5. You assume buffers are in memory, but do not require it. You
> should set r->filter_need_in_memory in header filter handler.
More information about the nginx-devel