Dnia czwartek 07 styczeń 2010 o 00:39:14 Maxim Dounin napisał(a):
> Hello!
>
> 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.
The header is a javascript with adverts.
> 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.
Thanks!
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://nginx.org/mailman/listinfo/nginx-devel
