a) gcc warnings: mod_xml2enc.c: In function 'fix_skipto': mod_xml2enc.c:123:18: warning: variable 'rv' set but not used [-Wunused-but-set-variable] mod_xml2enc.c: In function 'sniff_encoding': mod_xml2enc.c:167:18: warning: variable 'rv' set but not used [-Wunused-but-set-variable]
b) code style => http://httpd.apache.org/dev/styleguide.html e.g. "request_rec *r" not "request_rec* r" throughout running the thing through indent might help but it will require manual work too no doubt. d) This part which is supposed to cope with a brigade of non-determinate length doesn't look right - such a brigade is likely to contain a bucket type for which ->setaside will fail, so, you can't expect it will succeed: /* not enough data to sniff. Wait for more */ APR_BRIGADE_DO(b, ctx->bbsave) { apr_bucket_setaside(b, f->r->pool); } e) no error handling in various places: ap_fwrite(f->next, ctx->bbnext, ctx->buf, (apr_size_t)ctx->bblen - ctx->bytes); f) dubious cast: rv = apr_bucket_read(b, (const char**)&buf, &bytes, APR_BLOCK_READ); the returned pointer from ->read is const for a reason; e.g. for an IMMORTAL bucket, it will be in unwritable memory; this code seems to assume it is writable. joe
