Re: Httpd 3.0 or something else

[Basant Kukreja]

On Tue, Nov 10, 2009 at 05:30:34PM -0500, Akins, Brian wrote:
 On 11/10/09 1:56 PM, Greg Stein gst...@gmail.com wrote:
 
 
  But some buckets might be performing gzip or SSL encryption. That
  consumes CPU within the network thread.
 
 You could just run x times CPU cores number of network threads.  You can't
 use more than 100% of a CPU anyway.
 
 The model that some of us discussed -- Greg, you may have invented it ;) --
 was to have a small pool of acceptor threads (maybe just one) and a pool of
 worker threads. The acceptor threads accept connections and move them into
 worker threads - that's it.  A single fd is then entirely owned by that
 worker thread until it (the fd) goes away - network/disk io, gzip, ssl, etc.
Sun Web Server (originated from Netscape) (Also Open Web Server) currently
handle this way.  It has a pool of acceptor threads which accepts connections,
acceptor threads pushes the connection to a connection queue, worker threads
pulls connection from connection queue and serves the request.  Keep alive
daemon is also multi-threaded. So multiple keep alive threads polls for the
various sets of connection for future HTTP requests.  The above architecture is
highly scalable. Recently Sun published a specweb record using this Web Server
for 128 CMT threads (32 cores system).
http://www.spec.org/web2005/results/res2009q4/web2005-20091013-00143.txt

You can see the sources from Open Web Server code if you are interested.
http://wikis.sun.com/display/wsFOSS/Open+Web+Server

Regards,
Basant.

Re: svn commit: r691418 [1/2] - in /httpd/httpd/trunk: ./ docs/manual/mod/ modules/filters/

[Basant Kukreja]

Hi,

Attached is the *rough* patch which uses transient buckets in mod_sed output
filter.

Testing :
  I created a 30MB and 300MB text files and ran OutputSed commands on the file.
* Before the patch, process size (worker mpm with 1 thread) increased up to 
300M 
for single request.  After the  patch, process size remains to 3MB to server
300M response output.

I also removed 1 extra copying for processing output.

I need to add some more error handling to finalize the patch. Any comments are
welcome.

Regards,
Basant.

On Thu, Sep 04, 2008 at 09:47:26PM -0500, William A. Rowe, Jr. wrote:
 Basant Kukreja wrote:

 Based on your suggestion, I will check what are the other improvements from
 mod_substitute can be brought into mod_sed.

 Note that mod_substitute's brigade handling is already based on the work of
 both Jim and Nick (author of mod_line_edit) - so they are pretty certain
 that it is the right approach.  Good idea to borrow from it.

 Bill
Index: modules/filters/mod_sed.c
===
--- modules/filters/mod_sed.c   (revision 692768)
+++ modules/filters/mod_sed.c   (working copy)
@@ -26,7 +26,8 @@
 #include libsed.h
 
 static const char *sed_filter_name = Sed;
-#define MODSED_OUTBUF_SIZE 4000
+#define MODSED_OUTBUF_SIZE 8000
+#define MAX_TRANSIENT_BUCKETS 50
 
 typedef struct sed_expr_config
 {
@@ -44,11 +45,14 @@
 typedef struct sed_filter_ctxt
 {
 sed_eval_t eval;
+ap_filter_t *f;
 request_rec *r;
 apr_bucket_brigade *bb;
 char *outbuf;
 char *curoutbuf;
 int bufsize;
+apr_pool_t *tpool;
+int numbuckets;
 } sed_filter_ctxt;
 
 module AP_MODULE_DECLARE_DATA sed_module;
@@ -71,29 +75,68 @@
 sed_cfg-last_error = error;
 }
 
+/* clear the temporary pool (used for transient buckets)
+ */
+static void clear_ctxpool(sed_filter_ctxt* ctx)
+{
+apr_pool_clear(ctx-tpool);
+ctx-outbuf = NULL;
+ctx-curoutbuf = NULL;
+ctx-numbuckets = 0;
+}
+
+/* alloc_outbuf
+ * allocate output buffer
+ */
+static void alloc_outbuf(sed_filter_ctxt* ctx)
+{
+ctx-outbuf = apr_palloc(ctx-tpool, ctx-bufsize + 1);
+ctx-curoutbuf = ctx-outbuf;
+}
+
+/* append_bucket
+ * Allocate a new bucket from buf and sz and append to ctx-bb
+ */
+static void append_bucket(sed_filter_ctxt* ctx, char* buf, int sz)
+{
+int rv;
+apr_bucket *b;
+if (ctx-tpool == ctx-r-pool) {
+/* We are not using transient bucket */
+b = apr_bucket_pool_create(buf, sz, ctx-r-pool,
+   ctx-r-connection-bucket_alloc);
+APR_BRIGADE_INSERT_TAIL(ctx-bb, b);
+}
+else {
+/* We are using transient bucket */
+b = apr_bucket_transient_create(buf, sz,
+ctx-r-connection-bucket_alloc);
+APR_BRIGADE_INSERT_TAIL(ctx-bb, b);
+ctx-numbuckets++;
+if (ctx-numbuckets = MAX_TRANSIENT_BUCKETS) {
+b = apr_bucket_flush_create(ctx-r-connection-bucket_alloc);
+APR_BRIGADE_INSERT_TAIL(ctx-bb, b);
+rv = ap_pass_brigade(ctx-f-next, ctx-bb);
+apr_brigade_cleanup(ctx-bb);
+clear_ctxpool(ctx);
+}
+}
+}
+
 /*
  * flush_output_buffer
  * Flush the  output data (stored in ctx-outbuf)
  */
-static void flush_output_buffer(sed_filter_ctxt *ctx, char* buf, int sz)
+static void flush_output_buffer(sed_filter_ctxt *ctx)
 {
 int size = ctx-curoutbuf - ctx-outbuf;
 char *out;
-apr_bucket *b;
-if (size + sz = 0)
+if ((ctx-outbuf == NULL) || (size =0))
 return;
-out = apr_palloc(ctx-r-pool, size + sz);
-if (size) {
-memcpy(out, ctx-outbuf, size);
-}
-if (buf  (sz  0)) {
-memcpy(out + size, buf, sz);
-}
-/* Reset the output buffer position */
+out = apr_palloc(ctx-tpool, size);
+memcpy(out, ctx-outbuf, size);
+append_bucket(ctx, out, size);
 ctx-curoutbuf = ctx-outbuf;
-b = apr_bucket_pool_create(out, size + sz, ctx-r-pool,
-   ctx-r-connection-bucket_alloc);
-APR_BRIGADE_INSERT_TAIL(ctx-bb, b);
 }
 
 /* This is a call back function. When libsed wants to generate the output,
@@ -104,11 +147,38 @@
 /* dummy is basically filter context. Context is passed during invocation
  * of sed_eval_buffer
  */
+int remainbytes = 0;
 sed_filter_ctxt *ctx = (sed_filter_ctxt *) dummy;
-if (((ctx-curoutbuf - ctx-outbuf) + sz) = ctx-bufsize) {
-/* flush current buffer */
-flush_output_buffer(ctx, buf, sz);
+if (ctx-outbuf == NULL) {
+alloc_outbuf(ctx);
 }
+remainbytes = ctx-bufsize - (ctx-curoutbuf - ctx-outbuf);
+if (sz = remainbytes) {
+if (remainbytes  0) {
+memcpy(ctx-curoutbuf, buf, remainbytes);
+buf += remainbytes;
+sz -= remainbytes;
+ctx-curoutbuf += remainbytes;
+}
+/* buffer is now full */
+append_bucket(ctx, ctx

Re: svn commit: r691418 [2/2] - in /httpd/httpd/trunk: ./docs/manual/mod/ modules/filters/

[Basant Kukreja]

On Wed, Sep 10, 2008 at 10:10:20AM +0200, Plüm, Rüdiger, VF-Group wrote:
  
 
  -Ursprüngliche Nachricht-
  Von: [EMAIL PROTECTED] 
  Gesendet: Mittwoch, 10. September 2008 08:56
  An: dev@httpd.apache.org
  Betreff: Re: svn commit: r691418 [2/2] - in 
  /httpd/httpd/trunk: ./docs/manual/mod/ modules/filters/
  
 
  I investigated further. I wrote a test file having binary 
  character from 0 to 31:
  $ od -c out.txt
  000  \0  \n 001  \n 002  \n 003  \n 004  \n 005  \n 006  
  \n 007  \n
  020  \b  \n  \t  \n  \n  \n 013  \n  \f  \n  \r  \n 016  
  \n 017  \n
  ...
  
  And a small sed script :
   $ cat one.sed
  l
  d
  
  Sed script just runs the l command for each line.
  $ /usr/ucb/sed -f one.sed out.txt   out1.txt
  
  Here is the output of out1.txt
  $ od -c out1.txt
  000  \n   \   0   1  \n   \   0   2  \n   \   0   3  \n   
  \   0   4
  020  \n   \   0   5  \n   \   0   6  \n   \   0   7  \n   
  -  \b   
  040  \n   -  \b \n  \n  \n   \   1   3  \n   \   1   
  4  \n   \
  060   1   5  \n   \   1   6  \n   \   1   7  \n   \   2   
  0  \n   \
  ...
  
  $ cat out1.txt
  \01
  \02
  \03
  \04
  \05
  \06
  \07
  
  
  
  
  \13
  \14
  
  ---
  
  So for some strange reason :
  0x8 is converted to -\b and
  0x9 is converted to -\b
  
  That's what we see in trans variable.
  
  Do you think it could be a bug in original sed and should we 
  correct it? 
 
 I guess it is a bug in original sed and it should be corrected.
 IMHO it should be suffient to replace
 
 -\b
 
 and
 
 -\b
 
 with
 
 
 
 and
 
 
Can you elaborate why you chose  and . I could not think of any
reasons behind it.
sed's man page says :
(2)lList the pattern space on the standard  out-
 put  in  an  unambiguous  form. Non-printing
 characters are spelled in  two  digit  ASCII
 and long lines are folded.

So 0x8 and 0x9 char values, which I believe are non printable characters,
should be printed into *two* digit ASCII. So \10 and \11 looks to me
as conforming to man page.

Regards,
Basant.

Re: svn commit: r691418 [2/2] - in /httpd/httpd/trunk:./docs/manual/mod/ modules/filters/

[Basant Kukreja]

On Wed, Sep 10, 2008 at 03:41:21PM +0200, Plüm, Rüdiger, VF-Group wrote:
  
 
  -Ursprüngliche Nachricht-
  Von: [EMAIL PROTECTED] 
  Gesendet: Mittwoch, 10. September 2008 15:32
  An: dev@httpd.apache.org
  Betreff: Re: svn commit: r691418 [2/2] - in 
  /httpd/httpd/trunk:./docs/manual/mod/ modules/filters/
  
  On Wed, Sep 10, 2008 at 10:10:20AM +0200, Plüm, Rüdiger, 
  VF-Group wrote:

   
-Ursprüngliche Nachricht-
Von: [EMAIL PROTECTED] 
Gesendet: Mittwoch, 10. September 2008 08:56
An: dev@httpd.apache.org
Betreff: Re: svn commit: r691418 [2/2] - in 
/httpd/httpd/trunk: ./docs/manual/mod/ modules/filters/

   
I investigated further. I wrote a test file having binary 
character from 0 to 31:
$ od -c out.txt
000  \0  \n 001  \n 002  \n 003  \n 004  \n 005  \n 006  
\n 007  \n
020  \b  \n  \t  \n  \n  \n 013  \n  \f  \n  \r  \n 016  
\n 017  \n
...

And a small sed script :
 $ cat one.sed
l
d

Sed script just runs the l command for each line.
$ /usr/ucb/sed -f one.sed out.txt   out1.txt

Here is the output of out1.txt
$ od -c out1.txt
000  \n   \   0   1  \n   \   0   2  \n   \   0   3  \n   
\   0   4
020  \n   \   0   5  \n   \   0   6  \n   \   0   7  \n   
-  \b   
040  \n   -  \b \n  \n  \n   \   1   3  \n   \   1   
4  \n   \
060   1   5  \n   \   1   6  \n   \   1   7  \n   \   2   
0  \n   \
...

$ cat out1.txt
\01
\02
\03
\04
\05
\06
\07




\13
\14

---

So for some strange reason :
0x8 is converted to -\b and
0x9 is converted to -\b

That's what we see in trans variable.

Do you think it could be a bug in original sed and should we 
correct it? 
   
   I guess it is a bug in original sed and it should be corrected.
   IMHO it should be suffient to replace
   
   -\b
   
   and
   
   -\b
   
   with
   
   
   
   and
   
   
  Can you elaborate why you chose  and . I could not think of any
  reasons behind it.
 
 Because these characters are currently displayed by sed and I saw no reason 
 to change
 it.
 
  sed's man page says :
  (2)lList the pattern space on the standard  out-
   put  in  an  unambiguous  form. Non-printing
   characters are spelled in  two  digit  ASCII
   and long lines are folded.
  
  So 0x8 and 0x9 char values, which I believe are non printable 
  characters,
  should be printed into *two* digit ASCII. So \10 and \11 looks to me
  as conforming to man page.
 
 As said displaying two digit ASCII's also makes sense and conforms to the man
 page. So go for it.
 BTW: Shouldn't it be \08 and \09 for 0x8 and 0x9 instead of \10 and \11 ?
If we notice, how other characters are printed then sed code is using octal
numbers. After \17 it changes to \20, \21. Similarly After \27 it changes to
\30, \31 etc. So based on the above pattern, \10 and \11 seems to be more
consistent to me.

For reference, output of the converted characters (0-31) by l command is
given below.

$ cat out1.txt

\01
\02
\03
\04
\05
\06
\07




\13
\14
\15
\16
\17
\20
\21
\22
\23
\24
\25
\26
\27
\30
\31
\32
\33
\34
\35
\36
\37

I will submit the patch today. Thanks for noticing this.

Regards,
Basant.

Re: svn commit: r691418 [2/2] - in /httpd/httpd/trunk: ./ docs/manual/mod/ modules/filters/

[Basant Kukreja]

On Tue, Sep 09, 2008 at 10:10:42PM +0200, Ruediger Pluem wrote:


 On 09/03/2008 01:01 AM, [EMAIL PROTECTED] wrote:
 Added: httpd/httpd/trunk/modules/filters/sed1.c
 URL: 
 http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/sed1.c?rev=691418view=auto
 ==
 --- httpd/httpd/trunk/modules/filters/sed1.c (added)
 +++ httpd/httpd/trunk/modules/filters/sed1.c Tue Sep  2 16:01:47 2008
 @@ -0,0 +1,957 @@
 +/*
 + * Copyright (c) 2005, 2008 Sun Microsystems, Inc. All Rights Reserved.
 + * Use is subject to license terms.
 + *
 + *  Copyright (c) 1984 ATT
 + *All Rights Reserved   
 + *
 + * Licensed under the Apache License, Version 2.0 (the License);
 + * you may not use this file except in compliance with the License.
 + * You may obtain a copy of the License at
 + *  http://www.apache.org/licenses/LICENSE-2.0. + * + * Unless 
 required by applicable law or agreed to in writing, software + * 
 distributed under the License is distributed on an AS IS BASIS, + * 
 WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or 
 implied. + * See the License for the specific language governing 
 permissions and
 + * limitations under the License. + */
 +
 +#include apr.h
 +#include apr_lib.h
 +#include libsed.h
 +#include sed.h
 +#include apr_strings.h
 +#include regexp.h
 +
 +char*trans[040]  = {
 +\\01,
 +\\02,
 +\\03,
 +\\04,
 +\\05,
 +\\06,
 +\\07,
 +-,
 +-,

 What are the above constants supposed to be. Opening the file in vi shows 
 that they are special
 characters or better control characters. Looking with a hex editor these () 
 seem to be \\08.
 Is this correct?

Sed has a l command. From the sed man page :
 (2)lList the pattern space on the standard  out-
 put  in  an  unambiguous  form. Non-printing
 characters are spelled in  two  digit  ASCII
 and long lines are folded.

From the code :
   p3 = trans[(unsigned char)*p1-1];
while ((*p2++ = *p3++) != 0)
if(p2 = eval-lcomend) {
*p2 = '\\';
wline(eval, eval-genbuf, strlen(eval-genbuf));
p2 = eval-genbuf;
}


It looks to me that it is trying to print character from value 0 to 31 as
printable characters.

 +-,
 +-,
It seems to me that it should be \\08 and \\09.

I will dig deeper to see if these can be simplified. It looks little weird that
there are binary characters in source file.

Regards,
Basant.

Re: dtrace in apache

[Basant Kukreja]

Hi,
   I agree with Rahul. pid provider can be used to trace apr calls. Following
dscript traces apr calls in stock apache (without any probes)

pid*::apr_*:entry
/execname == httpd/
{
}

pid*::apr_*:return
/execname == httpd/
{
}
--
If apr calls would not have been started with prefix apr_ then putting a
specific probe might make more sense.

I have written a small blog for it.
http://blogs.sun.com/basant/entry/tracking_apr_calls_in_apache

Regards,
Basant.


On Sat, Aug 23, 2008 at 01:00:37PM +0530, rahul wrote:
 Hi,
 I was reviewing the omniti labs dtrace functions, at
 http://labs.omniti.com/trac/project-dtrace/browser/trunk/apache22/apr-util-hook-probes.patch
 This patch (util-hook) is committed into apache already.
 
 I was concerned that quite a few were just tracing function boundaries,
 which the dtrace does already for us with out the necessity of USDT probes
 baked into the code.
 
 For e.g in APR_IMPLEMENT_XXX macros, the below are inserted.
 but APR_IMPLEMENT_XXX macros already create function boundaries when
 they are called and these probes effectively duplicate the
 instrumentation available.
 
 
  /**
   * @file apr_hooks.h
   * @brief Apache hook functions
 @@ -107,12 +124,21 @@
  ns##_LINK_##name##_t *pHook; \
  int n; \
  \
 -if(!_hooks.link_##name) \
 -   return; \
 +OLD_DTRACE_PROBE(name##__entry); \
  \
 -pHook=(ns##_LINK_##name##_t *)_hooks.link_##name-elts; \
 -for(n=0 ; n  _hooks.link_##name-nelts ; ++n) \
 -   pHook[n].pFunc args_use; \
 +if(_hooks.link_##name) \
 +{ \
 +pHook=(ns##_LINK_##name##_t *)_hooks.link_##name-elts; \
 +for(n=0 ; n  _hooks.link_##name-nelts ; ++n) \
 +{ \
 +OLD_DTRACE_PROBE1(name##__dispatch__invoke, (char
 *)pHook[n].szName); \
 +   pHook[n].pFunc args_use; \
 +OLD_DTRACE_PROBE2(name##__dispatch__complete, (char
 *)pHook[n].szName, 0); \
 +} \
 +} \
 +\
 +OLD_DTRACE_PROBE1(name##__return, 0); \
 +\
  }
 
 
 What do you think?
 
 rahul
 --
 1. e4 _

Re: Dropping mod_sed into /trunk/ ?

[Basant Kukreja]

Hi,
   There is one important aspect of mod_sed design (which is borrowed from Sun
Web Server) that I would like to emphasize is that sed code has been separated
from filter code. So sed code itself can be archived in a shared/static library
(let me call it as libsed). mod_sed filter code is just one consumer of libsed.

Typical usage of libsed is :
sed_commands_t cmd
sed_init_commands(cmd, ...);
sed_compile_string(cmd, string1);
sed_compile_string(cmd, string2);
sed_finalize_command(cmd);

// Run eval on compiled context any number times.
sed_eval_t eval;
sed_init_eval(eval, ...);
sed_eval_buffer(eval, ...);
sed_finalize_eval(eval, ...);

// finally destroy the evaluation and compile context
sed_destroy_eval(eval);
sed_destroy_commands(cmd);


Having said that, I would like to emphasize the sed command and evaluation
context doesn't store any thread local storage. What this means is that
multiple commands and evalution context can be created within a single thread.
This means multiple consumers of libsed can be consumed safely in a single
thread.

This way, any number of apache modules can make use of sed libraries and
coexist in a same thread. If developers like this idea then one option is to
drop libsed code into apr and drop mod_sed.c (which is a sed filter module)
into apache code.

libsed code lies in sed0.c sed1.c and regexp.c regexp.h libsed.h
Whereas filter code is in mod_sed.c
http://src.opensolaris.org/source/xref/webstack/mod_sed/

Regards,
Basant.


On Wed, Aug 20, 2008 at 11:53:58PM +0100, Nick Kew wrote:
 A little while ago, Basant Kukreja published mod_sed under the
 Apache license.  He's now also written a blog entry that could
 become the basis for a tutorial into how mod_sed is much more
 than a mere string-or-regexp search-and-replace filter:
 
 http://blogs.sun.com/basant/entry/using_mod_sed_to_filter
 
 I happen to know that Basant and Sun will be happy for us
 to adopt mod_sed, and I think that with that blog entry
 reworked into a howto doc, it'll add real value to httpd.
 
 Any thoughts on dropping it in to trunk, with a view
 to including it as standard in 2.4 in due course?
 
 -- 
 Nick Kew
 
 Application Development with Apache - the Apache Modules Book
 http://www.apachetutor.org/

Re: Dropping mod_sed into /trunk/ ?

[Basant Kukreja]

On Sat, Aug 23, 2008 at 05:28:05PM +0530, rahul wrote:
 I have a request,
 
 in mod_sed, if there are multiline commands, specifying them is
 cumbersome, It would be nice to have the ability to specify a script
 file instead.
 
I agree. But I think mod_sed should support both. Any votes or comments?

E.g OutputSedScriptFile and InputSedScriptFile can read the sed commands from
sed script file.

libsed has a function to process file based request. It is matter of adding few
lines of code (additional configuration) in mod_sed to include this capability.

Regards,
Basant.

Re: Solaris sed based apache filtering module (mod_sed)

[Basant Kukreja]

Minor changes : Fixed remaining warnings :
* warning: array subscript has type 'char'
* warning: suggest parentheses around assignment used as truth value
* warning C4018: '' : signed/unsigned mismatch

Here is the diff :

diff -r 071789bf9134 regexp.c
--- a/regexp.c  Sun Jun 29 12:52:34 2008 -0700
+++ b/regexp.c  Fri Aug 08 17:49:35 2008 -0700
@@ -396,6 +396,7 @@ static int _advance(char *lp, char *ep, 
 char *bbeg;
 char neg;
 int ct;
+int epint; /* int value of *ep */
 
 while (1) {
 neg = 0;
@@ -440,11 +441,15 @@ static int _advance(char *lp, char *ep, 
 return (0);
 
 case CBRA:
-vars-braslist[*ep++] = lp;
+epint = (int) *ep;
+vars-braslist[epint] = lp;
+ep++;
 continue;
 
 case CKET:
-vars-braelist[*ep++] = lp;
+epint = (int) *ep;
+vars-braelist[epint] = lp;
+ep++;
 continue;
 
 case CCHR | RNGE:
@@ -516,8 +521,10 @@ static int _advance(char *lp, char *ep, 
 goto star;
 
 case CBACK:
-bbeg = vars-braslist[*ep];
-ct = vars-braelist[*ep++] - bbeg;
+epint = (int) *ep;
+bbeg = vars-braslist[epint];
+ct = vars-braelist[epint] - bbeg;
+ep++;
 
 if (ecmp(bbeg, lp, ct)) {
 lp += ct;
@@ -526,8 +533,10 @@ static int _advance(char *lp, char *ep, 
 return (0);
 
 case CBACK | STAR:
-bbeg = vars-braslist[*ep];
-ct = vars-braelist[*ep++] - bbeg;
+epint = (int) *ep;
+bbeg = vars-braslist[epint];
+ct = vars-braelist[epint] - bbeg;
+ep++;
 curlp = lp;
 while (ecmp(bbeg, lp, ct))
 lp += ct;
diff -r 071789bf9134 sed0.c
--- a/sed0.cSun Jun 29 12:52:34 2008 -0700
+++ b/sed0.cFri Aug 08 17:49:35 2008 -0700
@@ -343,7 +343,7 @@ swit:
 }
 *--tp = '\0';
 
-if (lpt = search(commands)) {
+if ((lpt = search(commands)) != NULL) {
 if (lpt-address) {
 command_errf(commands, SEDERR_DLMES, commands-linebuf);
 return -1;
@@ -437,8 +437,8 @@ jtcommon:
 commands-cp--;
 
 if (*commands-cp == '\0') {
-if (pt = commands-labtab-chain) {
-while (pt1 = pt-lb1)
+if ((pt = commands-labtab-chain) != NULL) {
+while ((pt1 = pt-lb1) != NULL)
 pt = pt1;
 pt-lb1 = commands-rep;
 } else
@@ -454,12 +454,12 @@ jtcommon:
 commands-cp--;
 *--tp = '\0';
 
-if (lpt = search(commands)) {
+if ((lpt = search(commands)) != NULL) {
 if (lpt-address) {
 commands-rep-lb1 = lpt-address;
 } else {
 pt = lpt-chain;
-while (pt1 = pt-lb1)
+while ((pt1 = pt-lb1) != NULL)
 pt = pt1;
 pt-lb1 = commands-rep;
 }
@@ -926,6 +926,7 @@ static char *ycomp(sed_commands_t *comma
 static char *ycomp(sed_commands_t *commands, char *expbuf)
 {
 charc;
+int cint; /* integer value of char c */
 char *ep, *tsp;
 int i;
 char*sp;
@@ -953,11 +954,12 @@ static char *ycomp(sed_commands_t *comma
 sp++;
 c = '\n';
 }
-if((ep[c] = *tsp++) == '\\'  *tsp == 'n') {
-ep[c] = '\n';
+cint = (int) c;
+if((ep[cint] = *tsp++) == '\\'  *tsp == 'n') {
+ep[cint] = '\n';
 tsp++;
 }
-if(ep[c] == commands-sseof || ep[c] == '\0') {
+if(ep[cint] == commands-sseof || ep[cint] == '\0') {
 command_errf(commands, SEDERR_TSNTSS, commands-linebuf);
 }
 }
diff -r 071789bf9134 sed1.c
--- a/sed1.cSun Jun 29 12:52:34 2008 -0700
+++ b/sed1.cFri Aug 08 17:49:35 2008 -0700
@@ -152,7 +152,7 @@ static void grow_gen_buffer(sed_eval_t *
  */
 static void appendmem_to_linebuf(sed_eval_t *eval, const char* sz, int len)
 {
-int reqsize = (eval-lspend - eval-linebuf) + len;
+unsigned int reqsize = (eval-lspend - eval-linebuf) + len;
 if (eval-lsize  reqsize) {
 grow_line_buffer(eval, reqsize);
 }
@@ -186,7 +186,7 @@ static void append_to_holdbuf(sed_eval_t
 static void append_to_holdbuf(sed_eval_t *eval, const char* sz)
 {
 int len = strlen(sz);
-int reqsize = (eval-hspend - eval-holdbuf) + len + 1;
+unsigned int reqsize = (eval-hspend - eval-holdbuf) + len + 1;
 if (eval-hsize = reqsize) {
 grow_hold_buffer(eval, reqsize);
 }
@@ -210,7 +210,7 @@ static void append_to_genbuf(sed_eval_t 
 static void append_to_genbuf(sed_eval_t *eval, 

Re: Solaris sed based apache filtering module (mod_sed)

[Basant . Kukreja]

Solaris ucb sed code works fine when each line has new line character. If new
line character is missing from the end of the file then behaviour was not
friendly. The reason is that sed is a true line editor.

$ cat nocrlf.inp
HTML
abcd
/HTML   --- no new line here
$ /usr/ucb/sed -e 's/a/g/g' nocrlf.inp
HTML
gbcd

gnu sed behaves properly in above situation.

This is the fix for the regression caused by my previous checkin. If last byte
of the file is not a new line, then last byte was not printing in to the
output.
Steffen [EMAIL PROTECTED] reported this issue to me.

diff is attached.

Regards,
Basant.

diff -r 2866cd8c83a2 sed1.c
--- a/sed1.cSat Jun 28 21:00:41 2008 -0700
+++ b/sed1.cSun Jun 29 12:51:49 2008 -0700
@@ -432,11 +432,10 @@ apr_status_t sed_finalize_eval(sed_eval_
 eval-lspend--;
 } else {
 /* Code can probably reach here when last character in output
- * buffer is not null terminated
+ * buffer is not a newline.
  */
 /* Assure space for NULL */
 append_to_linebuf(eval, );
-eval-lspend--;
 }

 *eval-lspend = '\0';


On Sat, Jun 28, 2008 at 09:08:50PM -0700, [EMAIL PROTECTED] wrote:
 mod_sed constant length buffer sizes fix :
 
 Three buffers linebuf, genbuf and holdbuf are now reallocated dynamically.
 Sed code has 3 buffers :
 linebuf : Store a line.
 holdbuf : store the intemediate hold data (sed h/H commands)
 genbuf : Used in substitution intermediate results.
 
 Original sed has the limitation on the fixed size buffers.
 In this fix all these 3 buffers are adjusted dynamically. Initial buffer size
 of these buffers is 1024 bytes. When these buffers needs reallocation, their
 new buffer size is aligned to 4KB boundary.
 
 List of affected files :
 sed1.c libsed.h
 
 Code has been updated and can be obtained by mercurial as :
 $ hg clone ssh://[EMAIL PROTECTED]/hg/webstack/mod_sed
 
 It should soon be visible at :
 http://src.opensolaris.org/source/xref/webstack/mod_sed/
 
 Diff is attached.
 
 Regards,
 Basant.
 
 
 -
 diff -r 32f5eb1dc14f libsed.h
 --- a/libsed.h  Wed Apr 30 11:52:57 2008 -0700
 +++ b/libsed.h  Sat Jun 28 20:35:06 2008 -0700
 @@ -121,17 +121,14 @@
  
  unsigned   lsize;
  char   *linebuf;
 -char   *lbend;
  char   *lspend;
  
  unsigned   hsize;
  char   *holdbuf;
 -char   *hbend;
  char   *hspend;
  
  unsigned   gsize;
  char   *genbuf;
 -char   *gbend;
  char   *lcomend;
  
  apr_file_t*fcode[NWFILES];
 diff -r 32f5eb1dc14f sed1.c
 --- a/sed1.cWed Apr 30 11:52:57 2008 -0700
 +++ b/sed1.cSat Jun 28 20:35:06 2008 -0700
 @@ -86,6 +86,151 @@
  }
  }
  
 +#define INIT_BUF_SIZE 1024
 +
 +/*
 + * grow_buffer
 + */
 +static void grow_buffer(apr_pool_t *pool, char **buffer,
 +char **spend, unsigned int *cursize,
 +unsigned int newsize)
 +{
 +char* newbuffer = NULL;
 +int spendsize = 0;
 +if (*cursize = newsize)
 +return;
 +/* Align it to 4 KB boundary */
 +newsize = (newsize  + ((1  12) - 1))  ~((1  12) -1);
 +newbuffer = apr_pcalloc(pool, newsize);
 +if (*spend  *buffer  (*cursize  0)) {
 +spendsize = *spend - *buffer;
 +}
 +if ((*cursize  0)  *buffer) {
 +memcpy(newbuffer, *buffer, *cursize);
 +}
 +*buffer = newbuffer;
 +*cursize = newsize;
 +if (spend != buffer) {
 +*spend = *buffer + spendsize;
 +}
 +}
 +
 +/*
 + * grow_line_buffer
 + */
 +static void grow_line_buffer(sed_eval_t *eval, int newsize)
 +{
 +grow_buffer(eval-pool, eval-linebuf, eval-lspend,
 +eval-lsize, newsize);
 +}
 +
 +/*
 + * grow_hold_buffer
 + */
 +static void grow_hold_buffer(sed_eval_t *eval, int newsize)
 +{
 +grow_buffer(eval-pool, eval-holdbuf, eval-hspend,
 +eval-hsize, newsize);
 +}
 +
 +/*
 + * grow_gen_buffer
 + */
 +static void grow_gen_buffer(sed_eval_t *eval, int newsize,
 +char **gspend)
 +{
 +if (gspend == NULL) {
 +gspend = eval-genbuf;
 +}
 +grow_buffer(eval-pool, eval-genbuf, gspend,
 +eval-gsize, newsize);
 +eval-lcomend = eval-genbuf[71];
 +}
 +
 +/*
 + * appendmem_to_linebuf
 + */
 +static void appendmem_to_linebuf(sed_eval_t *eval, const char* sz, int len)
 +{
 +int reqsize = (eval-lspend - eval-linebuf) + len;
 +if (eval-lsize  reqsize) {
 +grow_line_buffer(eval, reqsize);
 +}
 +memcpy(eval-lspend, sz, len);
 +eval-lspend += len;
 +}
 +
 +/*
 + * append_to_linebuf
 + */
 +static void append_to_linebuf(sed_eval_t *eval, const char* sz)
 +{
 +int len = strlen(sz);
 +/* Copy string including null character */
 +appendmem_to_linebuf(eval, sz, len + 1);
 +

Re: Solaris sed based apache filtering module (mod_sed)

[Basant . Kukreja]

mod_sed constant length buffer sizes fix :

Three buffers linebuf, genbuf and holdbuf are now reallocated dynamically.
Sed code has 3 buffers :
linebuf : Store a line.
holdbuf : store the intemediate hold data (sed h/H commands)
genbuf : Used in substitution intermediate results.

Original sed has the limitation on the fixed size buffers.
In this fix all these 3 buffers are adjusted dynamically. Initial buffer size
of these buffers is 1024 bytes. When these buffers needs reallocation, their
new buffer size is aligned to 4KB boundary.

List of affected files :
sed1.c libsed.h

Code has been updated and can be obtained by mercurial as :
$ hg clone ssh://[EMAIL PROTECTED]/hg/webstack/mod_sed

It should soon be visible at :
http://src.opensolaris.org/source/xref/webstack/mod_sed/

Diff is attached.

Regards,
Basant.


-
diff -r 32f5eb1dc14f libsed.h
--- a/libsed.h  Wed Apr 30 11:52:57 2008 -0700
+++ b/libsed.h  Sat Jun 28 20:35:06 2008 -0700
@@ -121,17 +121,14 @@
 
 unsigned   lsize;
 char   *linebuf;
-char   *lbend;
 char   *lspend;
 
 unsigned   hsize;
 char   *holdbuf;
-char   *hbend;
 char   *hspend;
 
 unsigned   gsize;
 char   *genbuf;
-char   *gbend;
 char   *lcomend;
 
 apr_file_t*fcode[NWFILES];
diff -r 32f5eb1dc14f sed1.c
--- a/sed1.cWed Apr 30 11:52:57 2008 -0700
+++ b/sed1.cSat Jun 28 20:35:06 2008 -0700
@@ -86,6 +86,151 @@
 }
 }
 
+#define INIT_BUF_SIZE 1024
+
+/*
+ * grow_buffer
+ */
+static void grow_buffer(apr_pool_t *pool, char **buffer,
+char **spend, unsigned int *cursize,
+unsigned int newsize)
+{
+char* newbuffer = NULL;
+int spendsize = 0;
+if (*cursize = newsize)
+return;
+/* Align it to 4 KB boundary */
+newsize = (newsize  + ((1  12) - 1))  ~((1  12) -1);
+newbuffer = apr_pcalloc(pool, newsize);
+if (*spend  *buffer  (*cursize  0)) {
+spendsize = *spend - *buffer;
+}
+if ((*cursize  0)  *buffer) {
+memcpy(newbuffer, *buffer, *cursize);
+}
+*buffer = newbuffer;
+*cursize = newsize;
+if (spend != buffer) {
+*spend = *buffer + spendsize;
+}
+}
+
+/*
+ * grow_line_buffer
+ */
+static void grow_line_buffer(sed_eval_t *eval, int newsize)
+{
+grow_buffer(eval-pool, eval-linebuf, eval-lspend,
+eval-lsize, newsize);
+}
+
+/*
+ * grow_hold_buffer
+ */
+static void grow_hold_buffer(sed_eval_t *eval, int newsize)
+{
+grow_buffer(eval-pool, eval-holdbuf, eval-hspend,
+eval-hsize, newsize);
+}
+
+/*
+ * grow_gen_buffer
+ */
+static void grow_gen_buffer(sed_eval_t *eval, int newsize,
+char **gspend)
+{
+if (gspend == NULL) {
+gspend = eval-genbuf;
+}
+grow_buffer(eval-pool, eval-genbuf, gspend,
+eval-gsize, newsize);
+eval-lcomend = eval-genbuf[71];
+}
+
+/*
+ * appendmem_to_linebuf
+ */
+static void appendmem_to_linebuf(sed_eval_t *eval, const char* sz, int len)
+{
+int reqsize = (eval-lspend - eval-linebuf) + len;
+if (eval-lsize  reqsize) {
+grow_line_buffer(eval, reqsize);
+}
+memcpy(eval-lspend, sz, len);
+eval-lspend += len;
+}
+
+/*
+ * append_to_linebuf
+ */
+static void append_to_linebuf(sed_eval_t *eval, const char* sz)
+{
+int len = strlen(sz);
+/* Copy string including null character */
+appendmem_to_linebuf(eval, sz, len + 1);
+--eval-lspend; /* lspend will now point to NULL character */
+}
+
+/*
+ * copy_to_linebuf
+ */
+static void copy_to_linebuf(sed_eval_t *eval, const char* sz)
+{
+eval-lspend = eval-linebuf;
+append_to_linebuf(eval, sz);
+}
+
+/*
+ * append_to_holdbuf
+ */
+static void append_to_holdbuf(sed_eval_t *eval, const char* sz)
+{
+int len = strlen(sz);
+int reqsize = (eval-hspend - eval-holdbuf) + len + 1;
+if (eval-hsize = reqsize) {
+grow_hold_buffer(eval, reqsize);
+}
+strcpy(eval-hspend, sz);
+/* hspend will now point to NULL character */
+eval-hspend += len;
+}
+
+/*
+ * copy_to_holdbuf
+ */
+static void copy_to_holdbuf(sed_eval_t *eval, const char* sz)
+{
+eval-hspend = eval-holdbuf;
+append_to_holdbuf(eval, sz);
+}
+
+/*
+ * append_to_genbuf
+ */
+static void append_to_genbuf(sed_eval_t *eval, const char* sz, char **gspend)
+{
+int len = strlen(sz);
+int reqsize = (*gspend - eval-genbuf) + len + 1;
+if (eval-gsize  reqsize) {
+grow_gen_buffer(eval, reqsize, gspend);
+}
+strcpy(*gspend, sz);
+/* *gspend will now point to NULL character */
+*gspend += len;
+}
+
+/*
+ * copy_to_genbuf
+ */
+static void copy_to_genbuf(sed_eval_t *eval, const char* sz)
+{
+int len = strlen(sz);
+int reqsize = len + 1;
+if (eval-gsize  reqsize) {
+grow_gen_buffer(eval, reqsize, NULL);
+

Re: [PATCH] DTrace probes patch.

[Basant Kukreja]

 I see no issues with making this the default and having a --disable-dtrace. 
  I can see a reason that someone might wish to turn them off -- thought 
 that someone isn't me.
+1
--disable-dtrace could be useful in certain scenarios e.g dtrace internal bugs.
IMHO, by default it should be enabled.

Regards,
Basant.

Re: Solaris sed based apache filtering module (mod_sed)

[Basant Kukreja]

Fixed the following 2 bugs :
1. ycomp may use uninitialized memory (This might have result in apache
   crash when used with y// sed commands).
2. Fixed windows compilation issue (Thanks to Steffen [EMAIL PROTECTED] for 
providing
   the patch).

List of affected files :
regexp.c mod_sed.c sed0.c

Code has been updated and can be obtained by mercurial as :
$ hg clone ssh://[EMAIL PROTECTED]/hg/webstack/mod_sed

It should soon be visible at :
http://src.opensolaris.org/source/xref/webstack/mod_sed/

Diff is attached.

Regards,
Basant.

--
diff -r 1a157e46cd86 mod_sed.c
--- a/mod_sed.c Thu Apr 24 17:26:08 2008 -0700
+++ b/mod_sed.c Wed Apr 30 11:50:54 2008 -0700
@@ -79,6 +79,7 @@ static void flush_output_buffer(sed_filt
 {
 int size = ctx-curoutbuf - ctx-outbuf;
 char *out;
+apr_bucket *b;
 if (size + sz = 0)
 return;
 out = apr_palloc(ctx-r-pool, size + sz);
@@ -90,8 +91,8 @@ static void flush_output_buffer(sed_filt
 }
 /* Reset the output buffer position */
 ctx-curoutbuf = ctx-outbuf;
-apr_bucket *b = apr_bucket_pool_create(out, size + sz, ctx-r-pool,
-   ctx-r-connection-bucket_alloc);
+b = apr_bucket_pool_create(out, size + sz, ctx-r-pool,
+   ctx-r-connection-bucket_alloc);
 APR_BRIGADE_INSERT_TAIL(ctx-bb, b);
 }

diff -r 1a157e46cd86 regexp.c
--- a/regexp.c  Thu Apr 24 17:26:08 2008 -0700
+++ b/regexp.c  Wed Apr 30 11:50:54 2008 -0700
@@ -307,7 +307,7 @@ char *sed_compile(sed_commands_t *comman
 if (cflg++)
 SEDCOMPILE_ERROR(44);
 if ((c = GETC()) == '\\')
-*ep++ = 255;
+*ep++ = (char) 255;
 else {
 UNGETC(c);
 goto nlim;
diff -r 1a157e46cd86 sed0.c
--- a/sed0.cThu Apr 24 17:26:08 2008 -0700
+++ b/sed0.cWed Apr 30 11:50:54 2008 -0700
@@ -68,7 +68,7 @@ apr_status_t sed_init_commands(sed_comma
 commands-lab = commands-labtab + 1;
 commands-pool = p;

-commands-respace = apr_palloc(p, RESIZE);
+commands-respace = apr_pcalloc(p, RESIZE);
 if (commands-respace == NULL) {
 command_errf(commands, SEDERR_OOMMES);
 return APR_EGENERAL;
@@ -945,6 +945,7 @@ static char *ycomp(sed_commands_t *comma
 }
 }
 tsp++;
+memset(ep, 0, 0400);

 while((c = *sp++) != commands-sseof) {
 c = 0377;

Re: IIS6 application pools feature in Apache..

[Basant Kukreja]

Sun Web Server also provides a feature in which a dedicated thread pool
could be created and certain part of application can be executed by this
thread pool. One application of such a feature is that if some
application is thread unsafe then users can create a thread pool of 1
thread and run that application in that pool. This will result in
synchronizing all calls to that application.

AFAIK there is no equivalent feature in apache today.

Regards,
Basant.

On Wed, Apr 30, 2008 at 11:36:58AM -0700, Ahab Abouzour wrote:
 
 Hello,
 
 IIS6 has a very useful feature called application pools, where you can 
 dedicate resources/worker processes per application. 
 
 Apache, until today, does not have such feature. Is there any plans to 
 implement this feature in future Apache releases. 
 
 Thanks!
 
 
   
 
 Be a better friend, newshound, and 
 know-it-all with Yahoo! Mobile.  Try it now.  
 http://mobile.yahoo.com/;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ

Re: Solaris sed based apache filtering module (mod_sed)

[Basant Kukreja]

I fixed one sed labels related bug. Sed labels were not been handled properly.

Code has been updated and can be obtained by mercurial as :
$ hg clone ssh://[EMAIL PROTECTED]/hg/webstack/mod_sed

Regards,
Basant.

diff -r 6dedc621b00d libsed.h
--- a/libsed.h  Thu Apr 17 15:15:01 2008 -0700
+++ b/libsed.h  Thu Apr 24 17:23:06 2008 -0700
@@ -104,6 +104,7 @@ struct sed_commands_s {
 sed_reptr_t  *rep;
 int  nrep;
 apr_pool_t   *pool;
+int  canbefinal;
 };

 typedef struct sed_eval_s sed_eval_t;
@@ -157,7 +158,8 @@ apr_status_t sed_init_commands(sed_comma
apr_pool_t *p);
 apr_status_t sed_compile_string(sed_commands_t *commands, const char *s);
 apr_status_t sed_compile_file(sed_commands_t *commands, apr_file_t *fin);
-apr_status_t sed_finalize_commands(sed_commands_t *commands);
+char* sed_get_finalize_error(const sed_commands_t *commands, apr_pool_t* pool);
+int sed_canbe_finalized(const sed_commands_t *commands);
 void sed_destroy_commands(sed_commands_t *commands);

 apr_status_t sed_init_eval(sed_eval_t *eval, sed_commands_t *commands,
diff -r 6dedc621b00d regexp.h
--- a/regexp.h  Thu Apr 17 15:15:01 2008 -0700
+++ b/regexp.h  Thu Apr 24 17:23:06 2008 -0700
@@ -102,6 +102,7 @@ extern void command_errf(sed_commands_t
 #define SEDERR_TSNTSS transform strings not the same size: %s
 #define SEDERR_OLTL output line too long.
 #define SEDERR_HSOVERFLOW hold space overflowed.
+#define SEDERR_INTERNAL internal sed error

 #ifdef __cplusplus
 }
diff -r 6dedc621b00d sed0.c
--- a/sed0.cThu Apr 17 15:15:01 2008 -0700
+++ b/sed0.cThu Apr 24 17:23:06 2008 -0700
@@ -39,6 +39,7 @@ static char *comple(sed_commands_t *comm
 static char *comple(sed_commands_t *commands, sed_comp_args *compargs,
 char *x1, char *ep, char *x3, char x4);
 static sed_reptr_t *alloc_reptr(sed_commands_t *commands);
+static int check_finalized(const sed_commands_t *commands);

 void command_errf(sed_commands_t *commands, const char *fmt, ...)
 {
@@ -80,6 +81,7 @@ apr_status_t sed_init_commands(sed_comma
 commands-rep-ad1 = commands-respace;
 commands-reend = commands-respace[RESIZE - 1];
 commands-labend = commands-labtab[SED_LABSIZE];
+commands-canbefinal = 1;

 return APR_SUCCESS;
 }
@@ -102,6 +104,8 @@ apr_status_t sed_compile_string(sed_comm
 commands-eflag = 1;

 rv = fcomp(commands, NULL);
+if (rv == APR_SUCCESS)
+commands-canbefinal = check_finalized(commands);

 commands-eflag = 0;

@@ -118,40 +122,75 @@ apr_status_t sed_compile_file(sed_comman
 }

 /*
- * sed_finalize_commands
- */
-apr_status_t sed_finalize_commands(sed_commands_t *commands)
-{
-sed_label_t *lab;
-
+ * sed_get_finalize_error
+ */
+char* sed_get_finalize_error(const sed_commands_t *commands, apr_pool_t* pool)
+{
+const sed_label_t *lab;
 if (commands-depth) {
-command_errf(commands, SEDERR_TMOMES);
-return APR_EGENERAL;
-}
-
-for (lab = commands-labtab; lab  commands-lab; lab++) {
+return SEDERR_TMOMES;
+}
+
+/* Empty branch chain is not a issue */
+for (lab = commands-labtab + 1; lab  commands-lab; lab++) {
+char *error;
 if (lab-address == 0) {
-command_errf(commands, SEDERR_ULMES, lab-asc);
-return APR_EGENERAL;
+error = apr_psprintf(pool, SEDERR_ULMES, lab-asc);
+return error;
 }

 if (lab-chain) {
-sed_reptr_t *rep;
-
-rep = lab-chain;
-while (rep-lb1) {
-sed_reptr_t *next;
-
-next = rep-lb1;
-rep-lb1 = lab-address;
-rep = next;
-}
-
-rep-lb1 = lab-address;
-}
-}
-
-return APR_SUCCESS;
+return SEDERR_INTERNAL;
+}
+}
+return NULL;
+}
+
+/*
+ * sed_canbe_finalized
+ */
+int sed_canbe_finalized(const sed_commands_t *commands)
+{
+return commands-canbefinal;
+}
+
+/*
+ * check_finalized
+ */
+static int check_finalized(const sed_commands_t *commands)
+{
+const sed_label_t *lab;
+if (commands-depth) {
+return 0;
+}
+
+/* Empty branch chain is not a issue */
+for (lab = commands-labtab + 1; lab  commands-lab; lab++) {
+if (lab-address == 0 || (lab-chain)) {
+return 0;
+}
+}
+return 1;
+}
+
+/*
+ * dechain
+ */
+static void dechain(sed_label_t *lpt, sed_reptr_t *address)
+{
+sed_reptr_t *rep;
+if ((lpt == NULL) || (lpt-chain == NULL) || (address == NULL))
+return;
+rep = lpt-chain;
+while (rep-lb1) {
+sed_reptr_t *next;
+
+next = rep-lb1;
+rep-lb1 = address;
+rep = next;
+}
+rep-lb1 = address;
+lpt-chain = NULL;
 }

 /*
@@ -309,6 +348,7 @@ swit:
 command_errf(commands, SEDERR_DLMES, commands-linebuf);
 return -1;
 }
+dechain(lpt, 

Re: Solaris sed based apache filtering module (mod_sed)

[Basant . Kukreja]

On Sat, Apr 12, 2008 at 10:27:40AM -0700, Chris Elving wrote:
 sed is an inherently line-oriented editor. It seems wrong to buffer 
 multiple lines within libsed. Consider, for example, how adding such 
 multi-line buffering to libsed would complicate implementing an 
 interactive sed like sed(1).
 
 Instead, it seems like such buffering should occur in mod_sed. mod_sed 
 is what couples libsed to the bucket brigade, and mod_sed knows that 
 such such buffering is appropriate.

As suggested by Chris Elving, I moved the buffering code from sed code to
mod_sed filter code.

Diff from first version is attached.
New sources can be viewed at :
http://src.opensolaris.org/source/xref/webstack/mod_sed/
(as well as by mercurial : hg clone ssh://[EMAIL PROTECTED]/hg/webstack/mod_sed 
)

Regards,
Basant.


Index: mod_sed.c
===
--- mod_sed.c   4 Apr 2008 02:33:57 -   1.20.2.6
+++ mod_sed.c   17 Apr 2008 21:33:01 -  1.20.2.8
@@ -26,6 +26,7 @@
 #include libsed.h
 
 static const char *sed_filter_name = Sed;
+#define MODSED_OUTBUF_SIZE 4000
 
 typedef struct sed_expr_config
 {
@@ -45,6 +46,9 @@
 sed_eval_t eval;
 request_rec *r;
 apr_bucket_brigade *bb;
+char *outbuf;
+char *curoutbuf;
+int bufsize;
 } sed_filter_ctxt;
 
 module AP_MODULE_DECLARE_DATA sed_module;
@@ -67,10 +71,32 @@
 sed_cfg-last_error = error;
 }
 
+/*
+ * flush_output_buffer
+ * Flush the  output data (stored in ctx-outbuf)
+ */
+static void flush_output_buffer(sed_filter_ctxt *ctx, char* buf, int sz)
+{
+int size = ctx-curoutbuf - ctx-outbuf;
+char *out;
+if (size + sz = 0)
+return;
+out = apr_palloc(ctx-r-pool, size + sz);
+if (size) {
+memcpy(out, ctx-outbuf, size);
+}
+if (buf  (sz  0)) {
+memcpy(out + size, buf, sz);
+}
+/* Reset the output buffer position */
+ctx-curoutbuf = ctx-outbuf;
+apr_bucket *b = apr_bucket_pool_create(out, size + sz, ctx-r-pool,
+   ctx-r-connection-bucket_alloc);
+APR_BRIGADE_INSERT_TAIL(ctx-bb, b);
+}
+
 /* This is a call back function. When libsed wants to generate the output,
- * this function will be invoked. Caller creates a copy of the buffer from the
- * request pool so we don't need to create a copy here. In this method, we
- * simply create a bucket and append into the brigade at the tail.
+ * this function will be invoked.
  */
 static void sed_write_output(void *dummy, char *buf, int sz)
 {
@@ -78,9 +104,14 @@
  * of sed_eval_buffer
  */
 sed_filter_ctxt *ctx = (sed_filter_ctxt *) dummy;
-apr_bucket *b = apr_bucket_pool_create(buf, sz, ctx-r-pool,
-   ctx-r-connection-bucket_alloc);
-APR_BRIGADE_INSERT_TAIL(ctx-bb, b);
+if (((ctx-curoutbuf - ctx-outbuf) + sz) = ctx-bufsize) {
+/* flush current buffer */
+flush_output_buffer(ctx, buf, sz);
+}
+else {
+memcpy(ctx-curoutbuf, buf, sz);
+ctx-curoutbuf += sz;
+}
 }
 
 /* Compile a sed expression. Compiled context is saved in sed_cfg-sed_cmds.
@@ -119,6 +150,34 @@
 return APR_SUCCESS;
 }
 
+/* Initialize sed filter context. If successful then context is set in f-ctx
+ */
+static apr_status_t init_context(ap_filter_t *f, sed_expr_config *sed_cfg)
+{
+apr_status_t status;
+sed_filter_ctxt* ctx;
+request_rec *r = f-r;
+/* Create the context. Call sed_init_eval. libsed will generated
+ * output by calling sed_write_output and generates any error by
+ * invoking log_sed_errf.
+ */
+ctx = apr_pcalloc(r-pool, sizeof(sed_filter_ctxt));
+ctx-r = r;
+ctx-bb = NULL;
+status = sed_init_eval(ctx-eval, sed_cfg-sed_cmds, log_sed_errf,
+   r, sed_write_output, r-pool);
+if (status != APR_SUCCESS) {
+return status;
+}
+apr_pool_cleanup_register(r-pool, ctx-eval, sed_eval_cleanup,
+  apr_pool_cleanup_null);
+ctx-bufsize = MODSED_OUTBUF_SIZE;
+ctx-outbuf = apr_palloc(r-pool, ctx-bufsize + 1);
+ctx-curoutbuf = ctx-outbuf;
+f-ctx = ctx;
+return APR_SUCCESS;
+}
+
 /* Entry function for Sed output filter */
 static apr_status_t sed_response_filter(ap_filter_t *f,
 apr_bucket_brigade *bb)
@@ -144,21 +203,10 @@
 return ap_pass_brigade(f-next, bb);
 }
 
-/* Create the context. Call sed_init_eval. libsed will generated
- * output by calling sed_write_output and generates any error by
- * invoking log_sed_errf.
- */
-ctx = f-ctx = apr_pcalloc(f-r-pool, sizeof(sed_filter_ctxt));
-ctx-r = f-r;
-ctx-bb = NULL;
-
-status = sed_init_eval(ctx-eval, sed_cfg-sed_cmds, log_sed_errf,
-   f-r, sed_write_output, f-r-pool);
-if (status != APR_SUCCESS) {
-return status;
-}
-   

Re: Solaris sed based apache filtering module (mod_sed)

[Basant Kukreja]

On Sat, Apr 12, 2008 at 10:27:40AM -0700, Chris Elving wrote:
 Basant Kukreja wrote:
 +static void sed_write(sed_eval_t *eval, char *buf, int sz)
 +{
 +if (eval-curoutbuf + sz = eval-outbend) {
 +// flush current buffer
 +sed_flush_output_buffer(eval, buf, sz);
 +}
 +else {
 +memcpy(eval-curoutbuf, buf, sz);
 +eval-curoutbuf += sz;
 +}

 sed is an inherently line-oriented editor. It seems wrong to buffer 
 multiple lines within libsed. Consider, for example, how adding such 
 multi-line buffering to libsed would complicate implementing an interactive 
 sed like sed(1).

 Instead, it seems like such buffering should occur in mod_sed. mod_sed is 
 what couples libsed to the bucket brigade, and mod_sed knows that such such 
 buffering is appropriate.
sed_eval_buffer and sed_finalize_eval flush the output before returning. If
somebody want to implement interactive sed then one can send line vise line
data to sed_eval_buffer (but that seems like poor assumption). So it is still
possible to implement interactive sed code using these APIs. 

But still it makes more sense to me to move buffering code outside the libsed.
I will work towards moving buffering code outside the libsed to mod_sed filter
code.

Regards,
Basant.

Re: Solaris sed based apache filtering module (mod_sed)

[Basant . Kukreja]

On Sat, Apr 12, 2008 at 11:56:29AM -0400, Jim Jagielski wrote:
 
 On Apr 10, 2008, at 7:51 PM, Basant Kukreja wrote:
 Any more details in which situations you see memory leaks? The same  
 leaks
 should happen on trunk as well.
 Yes, I verified that it leaks with trunk version too.
 
 
 A little more info would be appreciated :)
 
Just create a file say
test2.html in htdocs/testsub/ directory which contains :
--
hello world.
repeat 500 times above line.
--
Alias /testsub/ /usr/local/apache2/htdocs/testsub/
LoadModule substitute_module modules/mod_substitute.so
Directory /usr/local/apache2/htdocs/testsub
Options FollowSymLinks
AllowOverride None
AddOutputFilter SUBSTITUTE html
Substitute s/hello/hi/n
/Directory

Now run the stress using ab :
ab -c 400 -n 40 http://hostname/testsub/test2.html

Notice the size of httpd process. It will keep increasing to several GBs.
I tested with worker mpm (httpd-2.2.6) 64 bit on Solaris x86.
IfModule worker.c
ListenBackLog 5
StartServers 2
ThreadLimit500
ThreadsPerChild500
MinSpareThreads100
MaxSpareThreads1000
ThreadsPerChild500
MaxClients 1000
MaxRequestsPerChild  0
/IfModule

Re: Solaris sed based apache filtering module (mod_sed)

[Basant Kukreja]

 Furthermore from a first glance of mod_sed I would guess that it runs into
 trouble with large static files. So could you test mod_sed with one of
 the simple cases above and a 3 GB file and monitor httpd's memory consumption
 in comparison to mod_substitute?
I did testing with large files. Memory consumption was not a problem with large
files but response time was a big problem.
The performance of mod_sed was terrible with large files. I was not buffering
the output data in sed code which was causing too many buckets creation. This
didn't cause much performance drop for small files but for big files, it made
a large difference. I introduced the 4K buffer and that's solved the problem.

For a 3M file :
download time : 255 seconds
After my changes :
download time : 0.5 seconds

I also tried with 300M file and response time/memory consumption was not bad.

I have revised the mod_sed code and new code can be retrieve from mercurial :
$ hg clone ssh://[EMAIL PROTECTED]/hg/webstack/mod_sed

It should soon be visible on http url too.
http://src.opensolaris.org/source/xref/webstack/mod_sed/

Diffs are attached at the bottom. With these new changes here are the
performance comparison results. The changes improved performance in general.

Performance result with changes in mod_sed and trunk version of mod_substitute.
Apache : 2.2.6 64 bit worker mpm.
OS : Solaris snv_79
-
test2.html : 500 substitution s/hello/hi/g.
Requests Per Second (RPS) as measured by ab for s/hello/hi/ substitution.
Enabled CPUs : 4
testname   Concur  MaxReq  mod_sedmod_line_edit   mod_substitute  
mod_substitute
RPS RPS   RPS (with 
n flag) RPS
test2.html   400   40   2788 1363  2557 
2811

Summary :
On simple string substitution (s/hello/hi/) mod_sed perform better than
mod_substitue without [n] parameter and it is close to mod_substitute with [n]
parameter.
-
Test case summary : test for simple regular expression.
Regular expression used : OutputSed 's/.*one.*two/1 2/'
: Substitute s/.*one.*two/1 2/
String : hello one 1 two 2 three 3 four 4 five 5.
Enabled CPUs : 4
test22.html : 500 lines containing the above string (Result in 500 
substitution).
testname   Concur  MaxReq  mod_sedmod_substitute  
RPS RPS  
test22.html 40040   21681688
Summary :
With ordinary regular expression, mod_sed performed better than
mod_substitute.
-
Test case summary : pattern capture
Regular expression used : OutputSed 
's/.*\(one\).*\(two\).*\(three\).*\(four\).*\(five\).*/\5 \4 \3 \2 \1/'
: Substitute 
s/.*(one).*(two).*(three).*(four).*(five).*/$5 $4 $3 $2 $1/
String : hello one 1 two 2 three 3 four 4 five 5.
Enabled CPUs : 4
test12.html : 500 lines containing the above string (Result in 500 
substitution).
testname   Concur  MaxReq  mod_sedmod_substitute  
RPS RPS  
test12.html 40040   15233135
Summary :
With pattern capture, mod_substitute performed much better than mod_sed.
-

Regards,
Basant.

diff -r b16bfdb8759b libsed.h
--- a/libsed.h  Fri Apr 11 15:00:36 2008 -0700
+++ b/libsed.h  Fri Apr 11 15:04:44 2008 -0700
@@ -133,6 +133,10 @@ struct sed_eval_s {
 char   *gbend;
 char   *lcomend;
 
+char   *outbuf;
+char   *curoutbuf;
+char   *outbend;
+
 apr_file_t*fcode[NWFILES];
 sed_reptr_t*abuf[SED_ABUFSIZE];
 sed_reptr_t**aptr;
diff -r b16bfdb8759b sed1.c
--- a/sed1.cFri Apr 11 15:00:36 2008 -0700
+++ b/sed1.cFri Apr 11 15:04:37 2008 -0700
@@ -73,6 +73,8 @@ static apr_status_t command(sed_eval_t *
 step_vars_storage *step_vars);
 static void wline(sed_eval_t *eval, char *buf, int sz);
 static void arout(sed_eval_t *eval);
+static void sed_write(sed_eval_t *eval, char *buf, int sz);
+static void sed_flush_output_buffer(sed_eval_t *eval, char* buf, int sz);
 
 static void eval_errf(sed_eval_t *eval, const char *fmt, ...)
 {
@@ -124,6 +126,13 @@ apr_status_t sed_reset_eval(sed_eval_t *
 eval-genbuf = apr_palloc(eval-pool, LBSIZE + 1);
 eval-gbend = eval-genbuf + LBSIZE;
 }
+if (eval-outbuf == NULL) {
+int bufsize = 4 * LBSIZE;
+eval-outbuf = apr_palloc(eval-pool, bufsize + 1);
+eval-outbend = eval-outbuf + bufsize;
+}
+eval-curoutbuf = eval-outbuf;
+
 eval-lspend = eval-linebuf;
 eval-hspend = eval-holdbuf;
 eval-lcomend = eval-genbuf[71];
@@ -253,6 +262,7 @@ 

Re: Solaris sed based apache filtering module (mod_sed)

[Basant Kukreja]

On Thu, Apr 10, 2008 at 07:27:15AM -0400, Jim Jagielski wrote:

 On Apr 9, 2008, at 5:09 PM, Basant Kukreja wrote:
 just that, too.  Any performance comparisons with its competitors?
 Some quick performance comparison results between mod_sed, mod_line_edit 
 and
 mod_substitute.

 Search/Replace string : s/hello/hi/
 test0.html : No matches, so no substitute.
 test.html  :  1 comparision/substitute
 test2.html : 500 comparision/substitute

 Performance data was generated with 200 clients for 10 requests (for 
 worker
 mpm).
 $ ab -c 200 -n 10 http-uri

 Requests Per Second (RPS) as measured by ab for s/hello/hi/ substitution.
 mod_sed  mod_line_edit   
 mod_substitute
 test0.html3263   3206   3129
 test.html 3229   3213   3197
 test2.html1278   4041130

 Note that mod_sed performs better than either mod_line_edit or 
 mod_substitute.

 Interesting... What versions of each did you test?
OS : solaris 64 bit x86 nevada snv_79
Apache : httpd 2.2.6 64 bit worker mpm. 
mod_substitute.c : From httpd-2.2.8.
mod_line_edit -- Version 1.0
Network connection : Dedicated 1 GB e1000g card.
Server : X4100

Regards,
Basant.

Re: Solaris sed based apache filtering module (mod_sed)

[Basant Kukreja]

Here is some more performance results based on Ruediger's suggestion :

-
test2.html : 500 substitution s/hello/hi/g.
Requests Per Second (RPS) as measured by ab for s/hello/hi/ substitution.
Enabled CPUs : 1
testname   Concur  MaxReq  mod_sedmod_line_edit   mod_substitute  
mod_substitute(with n flag)
RPS RPS   RPS
test2.html   100   10   1278 404  1130 
1531

Summary :
On simple string substitution (s/hello/hi/) mod_sed perform better than
mod_substitue but with flag [n] mod_substitute performed better than
mod_sed.
-
Test case summary : pattern capture
Regular expression used : OutputSed 
's/.*\(one\).*\(two\).*\(three\).*\(four\).*\(five\).*/\5 \4 \3 \2 \1/'
: Substitute 
s/.*(one).*(two).*(three).*(four).*(five).*/$5 $4 $3 $2 $1/
String : hello one 1 two 2 three 3 four 4 five 5.
Enabled CPUs : 4
test12.html : 500 lines containing the above string (Result in 500 
substitution).
testname   Concur  MaxReq  mod_sedmod_substitute  
RPS RPS  
test12.html 12510   13223230
Summary :
With pattern capture, mod_substitute performed much better than mod_sed.
-
Test case summary : test for simple regular expression.
Regular expression used : OutputSed 's/.*one.*two/1 2/'
: Substitute s/.*one.*two/1 2/
String : hello one 1 two 2 three 3 four 4 five 5.
Enabled CPUs : 4
test22.html : 500 lines containing the above string (Result in 500 
substitution).
testname   Concur  MaxReq  mod_sedmod_substitute  
RPS RPS  
test22.html 40040   20131689
Summary :
With ordinary regular expression, mod_sed performed better than
mod_substitute.
-

mod_substitute (what is available in 2.2.8 is leaking memory) so the above
performance results must be repeated with new mod_substitute from trunk (to
have better comparison).

Regards,
Basant.


On Thu, Apr 10, 2008 at 01:44:53PM +0200, Ruediger Pluem wrote:
 On 10.04.2008 13:27, Jim Jagielski wrote:

 On Apr 9, 2008, at 5:09 PM, Basant Kukreja wrote:
 just that, too.  Any performance comparisons with its competitors?
 Some quick performance comparison results between mod_sed, mod_line_edit 
 and
 mod_substitute.

 Search/Replace string : s/hello/hi/
 test0.html : No matches, so no substitute.
 test.html  :  1 comparision/substitute
 test2.html : 500 comparision/substitute

 Performance data was generated with 200 clients for 10 requests (for 
 worker
 mpm).
 $ ab -c 200 -n 10 http-uri

 Requests Per Second (RPS) as measured by ab for s/hello/hi/ substitution.
 mod_sed  mod_line_edit   
 mod_substitute
 test0.html3263   3206   3129
 test.html 3229   3213   3197
 test2.html1278   4041130

 Note that mod_sed performs better than either mod_line_edit or 
 mod_substitute.

 Interesting... What versions of each did you test?

 Would be interesting to know what happens speed wise if you use the n flag
 for mod_substitute to use strmatch instead of a regular expression.
 And depending on the version you used it would be interesting what happens
 if you use the q flag (applys only to trunk).
 Furthermore from a first glance of mod_sed I would guess that it runs into
 trouble with large static files. So could you test mod_sed with one of
 the simple cases above and a 3 GB file and monitor httpd's memory consumption
 in comparison to mod_substitute?

 Regards

 Rüdiger

Re: Solaris sed based apache filtering module (mod_sed)

[Basant Kukreja]

On Wed, Apr 09, 2008 at 04:32:09AM +0100, Nick Kew wrote:
 On 9 Apr 2008, at 03:40, [EMAIL PROTECTED] wrote:
 Hi,
 mod_sed is a sed[1] based Apache filtering module, with the sed code based 
 on
 opensolaris sed. sed scripting offers a more familiar interface to build
 powerful filters. This mod_sed is designed to work with Apache HTTP server
 version 2.2 and above.

 Thanks.  This is useful stuff.  Of course mod_sed overlaps with 
 mod_line_edit
 and mod_substitute, but you're the first to support both input and output
 filtering.  My knowledge of sed itself is limited to the same kind of 
 search-
 and-replace those filters offer, but I have an idea it does a little more 
 than
 just that, too.  Any performance comparisons with its competitors?
I have not yet done any performance comparisons with its competitor but feature
wise sed is very rich. I will try to do some basic performance testing and come
back with results. Besides search and replace and regular expressions, Some of
it's feature are :
1) One can write if/else condition using sed labels and branches.
If (match) do this 
else do that.
2) Read/Write to a file using 'r' and 'w' commands
3) operations on a particular line number.
4) deleting lines on a condition.
5) Using braces to specify multiple operations if a condition matches.
6) inserting and appending text
and many others features.

Here is one of the sed tutorial (from google)
http://www.grymoire.com/Unix/Sed.html


 Design of this filter is in the way that regexp.c, regexp.h, sed0.c 
 sed1.c,
 libsed.h can be made part of the apr. While the filter code mod_sed.c
 will be a filter module which can reside under modules directory. This
 way other modules can also take advantage of sed libraries.

 I think you mean you've adapted the sed code to run on APR pools,
 and updated it for thread-safety/reentrancy.  Once we have a
 modular build system in a release version of apr-util, that could
 perhaps fit nicely as an apr_sed module.  But that's a whole other
 discussion.

 Please have a look into the code and provide your comments.

 Well, I've had a preview of this, and it's good :-)

 -- 
 Nick Kew

Regards,
Basant.

Re: Solaris sed based apache filtering module (mod_sed)

[Basant Kukreja]

 just that, too.  Any performance comparisons with its competitors?
Some quick performance comparison results between mod_sed, mod_line_edit and
mod_substitute.

Search/Replace string : s/hello/hi/
test0.html : No matches, so no substitute.
test.html  :  1 comparision/substitute
test2.html : 500 comparision/substitute

Performance data was generated with 200 clients for 10 requests (for worker
mpm).
$ ab -c 200 -n 10 http-uri

Requests Per Second (RPS) as measured by ab for s/hello/hi/ substitution.
 mod_sed  mod_line_edit   mod_substitute
test0.html3263   3206   3129
test.html 3229   3213   3197 
test2.html1278   4041130

Note that mod_sed performs better than either mod_line_edit or mod_substitute.

Here is the configuration :
--
Alias /testsed/ /usr/local/apache2/htdocs/testsed/
LoadModule sed_module modules/mod_sed.so
Directory /usr/local/apache2/htdocs/testsed
Options FollowSymLinks
AllowOverride None
AddOutputFilter Sed html
OutputSed s/hello/hi/g
/Directory

Alias /testled/ /usr/local/apache2/htdocs/testled/
LoadModule line_edit_module modules/mod_line_edit.so
Directory /usr/local/apache2/htdocs/testled
Options FollowSymLinks
AddOutputFilter line-editor .html
AllowOverride None
LERewriteRule hello hi [R]
/Directory

Alias /testsub/ /usr/local/apache2/htdocs/testsub/
LoadModule substitute_module modules/mod_substitute.so
Directory /usr/local/apache2/htdocs/testsub
Options FollowSymLinks
AllowOverride None
AddOutputFilter SUBSTITUTE html
Substitute s/hello/hi/
/Directory
--

Regards,
Basant.

Solaris sed based apache filtering module (mod_sed)

[Basant . Kukreja]

Hi,
mod_sed is a sed[1] based Apache filtering module, with the sed code based on
opensolaris sed. sed scripting offers a more familiar interface to build
powerful filters. This mod_sed is designed to work with Apache HTTP server
version 2.2 and above.

The functionality is similar to sed based NSAPI IO filter available in Sun Web
Server 7.0 [2][3].

This code is being shared under Apache 2.0 license.  Functional specification
for the mod_sed is given in mod_sed_filter.html [4].

README [5] file contains the instructions how to compile/use mod_sed.

Design of this filter is in the way that regexp.c, regexp.h, sed0.c sed1.c,
libsed.h can be made part of the apr. While the filter code mod_sed.c
will be a filter module which can reside under modules directory. This
way other modules can also take advantage of sed libraries.

Source code is obtained by : 
hg clone ssh://[EMAIL PROTECTED]/hg/webstack/mod_sed

Source code is also availabe by http here :
http://src.opensolaris.org/source/xref/webstack/mod_sed/

Please have a look into the code and provide your comments.

Regards,
Basant.

Note : hg (mercurial) can be found at [6].

[1] http://docs.sun.com/app/docs/doc/816-5165/sed-1b?a=view
[2] http://docs.sun.com/app/docs/doc/819-2630/6n4thbj5e?a=view
[3] http://docs.sun.com/app/docs/doc/820-1062/6ncoqnq52?a=view#gbzoo
[4] http://src.opensolaris.org/source/raw/webstack/mod_sed/mod_sed_filter.html
[5] http://src.opensolaris.org/source/xref/webstack/mod_sed/README
[6] http://www.selenic.com/mercurial/wiki/index.cgi/UnixInstall

Question regarding apache and locale (use of mbtowc in apache modules)

[Basant Kukreja]

Apache typically runs in C/POSIX locale. Is it possible/recommended to run
apache in other locale? Why I am asking is that if some apache module wants to
use mbtowc functions (wchar.h), is it ok to use it (as it affects the entire
process)? Is it recommended? Will it cause any issues with other builtin apache
modules?

Apache code though never call the setlocale function so I think typically
apache runs in system's default locale.

Regards,
Basant.

[PATCH 30730] mod_actions and Server-Status (Patch review request).

[Basant Kukreja]

Hi,
   I am Basant and I work for Sun Microsystems Inc. in web tier group.

I have submitted the patch for 30730.
Bug Id : 30730 http://issues.apache.org/bugzilla/show_bug.cgi?id=30730
Summary : mod_actions and Server-Status 
Patch uri : http://issues.apache.org/bugzilla/attachment.cgi?id=19706

I would like to make a humble review request for the patch.

Thanks and Regards,
Basant.

[PATCH 11035] Patch review request

[Basant Kukreja]

Hi,
   I am Basant. I work in web tier group of Sun Microsystems Inc.

Can some of the committers kindly review the patch for bug 11035 please?
Subject : Apache adds double entries to headers generated by CGI
URI : http://issues.apache.org/bugzilla/show_bug.cgi?id=11035


Thanks and Regards,
Basant.

Re: [PATCH 39299] - Revised Patch review request

[Basant Kukreja]

Hi,
   I have revised the patch. Can some of the commiter kindly review
the patch?

Regards,
Basant.

On Fri, Mar 02, 2007 at 10:53:29AM -0800, Basant Kukreja wrote:
 On Fri, Mar 02, 2007 at 08:39:25AM +0100, Plüm, Rüdiger, VF EITO wrote:
  
  
   -Ursprüngliche Nachricht-
   Von: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] 
   Gesendet: Freitag, 2. März 2007 02:15
   An: dev@httpd.apache.org
   Betreff: Re: [PATCH 39299] - Patch review request
   
   
   Thanks Nick for responding to my request.
   
   My comments are in between.
   
   On Wed, Feb 28, 2007 at 10:49:48PM +, Nick Kew wrote:
On Wed, 28 Feb 2007 14:31:19 -0800
Basant Kukreja [EMAIL PROTECTED] wrote:

 Hi,
I am Basant. I work in web tier group in Sun Microsystems Inc.
 
 I have submitted the patch for bug 39299.
 Summary : Internal Server Error (500) on COPY
 URI : http://issues.apache.org/bugzilla/show_bug.cgi?id=39299
 
 
 Can some of the committer kindly review my patch please 
   to see if it
 is acceptable or not?
 Patch is against 2.2.x branch.

409 implies a condition the client can fix.  Your patch tests for
a particular condition that is likely to be fixable in a server
with DAV uprunning.  But AFAICS it could also give a bogus 409,
for example in the case of a newly-installed and misconfigured
server.
   Can you kindly elaborate more? How newly misconfigured server can
   send 409? Here is my test case :
   
   DavLockDB /disk/apache/apache2/var/DAVLockFs
   Directory /disk/apache/apache2/htdocs/DAVtest
   Options Indexes FollowSymLinks
   AllowOverride None
   order allow,deny
   allow from all
   AuthName SMA Development server
   AuthType Basic
   DAV On
   /Directory
   
   Now assuming, I misconfigured the server and I intended to 
   configure /DAVtest1 instead of
   /DAVtest, if I send a request.
   
   --
   COPY /DAVtest1/litmus/copysrc HTTP/1.1
   Host: myhostname.mydomain:4004
   User-Agent: litmus/0.11 neon/0.25.5
   Connection: TE
   TE: trailers
   Depth: 0
   Destination: 
   http://myhostname.mydomain:4004/DAVtest/litmus/nonesuch/foo
   Overwrite: F
   X-Litmus: copymove: 5 (copy_nodestcoll
  
  I guess Nicks Idea was the other way round:
  
  COPY /DAVtest/litmus/copysrc
  
  Destination: http://myhostname.mydomain:4004/DAVtest1/litmus/nonesuch/foo
  
 There is a crash issue in that case. I need to address the issue along
 with the current fix. I have update the PR.
 
 Regards,
 Basant.
 
  IMHO this direction would also better match the problem described in 
  PR39299.
  
  Regards
  
  Rüdiger
  
  

Re: [PATCH 39299] - Patch review request

[Basant Kukreja]

On Fri, Mar 02, 2007 at 08:39:25AM +0100, Plüm, Rüdiger, VF EITO wrote:
 
 
  -Ursprüngliche Nachricht-
  Von: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] 
  Gesendet: Freitag, 2. März 2007 02:15
  An: dev@httpd.apache.org
  Betreff: Re: [PATCH 39299] - Patch review request
  
  
  Thanks Nick for responding to my request.
  
  My comments are in between.
  
  On Wed, Feb 28, 2007 at 10:49:48PM +, Nick Kew wrote:
   On Wed, 28 Feb 2007 14:31:19 -0800
   Basant Kukreja [EMAIL PROTECTED] wrote:
   
Hi,
   I am Basant. I work in web tier group in Sun Microsystems Inc.

I have submitted the patch for bug 39299.
Summary : Internal Server Error (500) on COPY
URI : http://issues.apache.org/bugzilla/show_bug.cgi?id=39299


Can some of the committer kindly review my patch please 
  to see if it
is acceptable or not?
Patch is against 2.2.x branch.
   
   409 implies a condition the client can fix.  Your patch tests for
   a particular condition that is likely to be fixable in a server
   with DAV uprunning.  But AFAICS it could also give a bogus 409,
   for example in the case of a newly-installed and misconfigured
   server.
  Can you kindly elaborate more? How newly misconfigured server can
  send 409? Here is my test case :
  
  DavLockDB /disk/apache/apache2/var/DAVLockFs
  Directory /disk/apache/apache2/htdocs/DAVtest
  Options Indexes FollowSymLinks
  AllowOverride None
  order allow,deny
  allow from all
  AuthName SMA Development server
  AuthType Basic
  DAV On
  /Directory
  
  Now assuming, I misconfigured the server and I intended to 
  configure /DAVtest1 instead of
  /DAVtest, if I send a request.
  
  --
  COPY /DAVtest1/litmus/copysrc HTTP/1.1
  Host: myhostname.mydomain:4004
  User-Agent: litmus/0.11 neon/0.25.5
  Connection: TE
  TE: trailers
  Depth: 0
  Destination: 
  http://myhostname.mydomain:4004/DAVtest/litmus/nonesuch/foo
  Overwrite: F
  X-Litmus: copymove: 5 (copy_nodestcoll
 
 I guess Nicks Idea was the other way round:
 
 COPY /DAVtest/litmus/copysrc
 
 Destination: http://myhostname.mydomain:4004/DAVtest1/litmus/nonesuch/foo
 
There is a crash issue in that case. I need to address the issue along
with the current fix. I have update the PR.

Regards,
Basant.

 IMHO this direction would also better match the problem described in PR39299.
 
 Regards
 
 Rüdiger
 
 

Re: [PATCH 39299] - Patch review request

[Basant Kukreja]

Thanks Nick for responding to my request.

My comments are in between.

On Wed, Feb 28, 2007 at 10:49:48PM +, Nick Kew wrote:
 On Wed, 28 Feb 2007 14:31:19 -0800
 Basant Kukreja [EMAIL PROTECTED] wrote:
 
  Hi,
 I am Basant. I work in web tier group in Sun Microsystems Inc.
  
  I have submitted the patch for bug 39299.
  Summary : Internal Server Error (500) on COPY
  URI : http://issues.apache.org/bugzilla/show_bug.cgi?id=39299
  
  
  Can some of the committer kindly review my patch please to see if it
  is acceptable or not?
  Patch is against 2.2.x branch.
 
 409 implies a condition the client can fix.  Your patch tests for
 a particular condition that is likely to be fixable in a server
 with DAV uprunning.  But AFAICS it could also give a bogus 409,
 for example in the case of a newly-installed and misconfigured
 server.
Can you kindly elaborate more? How newly misconfigured server can
send 409? Here is my test case :

DavLockDB /disk/apache/apache2/var/DAVLockFs
Directory /disk/apache/apache2/htdocs/DAVtest
Options Indexes FollowSymLinks
AllowOverride None
order allow,deny
allow from all
AuthName SMA Development server
AuthType Basic
DAV On
/Directory

Now assuming, I misconfigured the server and I intended to configure /DAVtest1 
instead of
/DAVtest, if I send a request.

--
COPY /DAVtest1/litmus/copysrc HTTP/1.1
Host: myhostname.mydomain:4004
User-Agent: litmus/0.11 neon/0.25.5
Connection: TE
TE: trailers
Depth: 0
Destination: http://myhostname.mydomain:4004/DAVtest/litmus/nonesuch/foo
Overwrite: F
X-Litmus: copymove: 5 (copy_nodestcoll)

--
I will get a 405 response.
--
HTTP/1.1 405 Method Not Allowed
Date: Thu, 01 Mar 2007 04:12:59 GMT
Server: Apache/2.2.5-dev (Unix) mod_ssl/2.2.5-dev OpenSSL/0.9.8a DAV/2 
SVN/1.4.3 mod_perl/2.0.4-dev Perl/v5.8.8
Allow: GET,HEAD,POST,OPTIONS,TRACE
Content-Length: 245
Content-Type: text/html; charset=iso-8859-1
!DOCTYPE HTML PUBLIC -//IETF//DTD HTML 2.0//EN
htmlhead
title405 Method Not Allowed/title
/headbody
h1Method Not Allowed/h1
pThe requested method COPY is not allowed for the URL 
/DAVtest1/litmus/copysrc./p
/body/html
--

 
 Does the DAV RFC explicitly tell us to use 409 in this instance?
I didn't find RFC explictly stating 409 response but it is one of the
responses returned by COPY method. I will dig more and return back on this.

Regards,
Basant.

 
 -- 
 Nick Kew
 
 Application Development with Apache - the Apache Modules Book
 http://www.apachetutor.org/

Re: [PATCH 38014] - Patch review request

[Basant Kukreja]

Revised patch after incorporating Will Rowe's suggestion.

Regards,
Basant.

On Tue, Feb 27, 2007 at 05:06:57PM -0800, Basant Kukreja wrote:
 Hi,
 I work in the web tier group of Sun Microsystems Inc. 
 
 I have submitted the patch for bug 38014
 (The status '100 Continue' will be sent after the final status code)
 http://issues.apache.org/bugzilla/show_bug.cgi?id=38014 
 
 Can some of the committer kindly review my patch please to see if it is
 acceptable or not?
 Patch is against 2.2.x branch.
 
 Regards,
 Basant.
 

[PATCH 39299] - Patch review request

[Basant Kukreja]

Hi,
   I am Basant. I work in web tier group in Sun Microsystems Inc.

I have submitted the patch for bug 39299.
Summary : Internal Server Error (500) on COPY
URI : http://issues.apache.org/bugzilla/show_bug.cgi?id=39299


Can some of the committer kindly review my patch please to see if it is
acceptable or not?
Patch is against 2.2.x branch.

Regards,
Basant.

[PATCH 38014] - Patch review request

[Basant Kukreja]

Hi,
I work in the web tier group of Sun Microsystems Inc. 

I have submitted the patch for bug 38014
(The status '100 Continue' will be sent after the final status code)
http://issues.apache.org/bugzilla/show_bug.cgi?id=38014 

Can some of the committer kindly review my patch please to see if it is
acceptable or not?
Patch is against 2.2.x branch.

Regards,
Basant.