[Patch] add -p option to override prefix

[Maxim Dounin]

Hello!

On Thu, Mar 26, 2009 at 01:27:21PM +0100, Jérôme Loyet wrote:

> > the previous patch (nginx-prefix-v2.patch) has to be applied after the
> > first one.
> >
> > I attach here the full patch.
> >
> > ++ jerome
> 
> In my path I use ngx_snprintf with "%s%s" but it seems not to work, I
> have overflows using this function.
> 
> I add tow logs to check the string content:
> 2009/03/26 11:56:34 [notice] 22943#0: pid file: ./logs/nginx.pid appq
> 2009/03/26 11:56:34 [notice] 22943#0: lock file: ./logs/nginx.lock   Y
> 
> The function ngx_s(n)printf does not work with "%s%s".

How do you log it?  If you use "%s" while logging it won't work - 
since nginx strings aren't normally null-terminated (and it looks 
for me that your code doesn't terminate them).  You should 
use %V instead, or %*s with explicitly specified length.

BTW, this may be an issue since in many places nginx assumes that 
variables from configuration directives *are* null-terminated.  
Probably you want to use ngx_snprintf("%s%s%Z") in your code 
instead, this will produce null-terminated string (%Z expands to 
'\0', see src/core/ngx_string.c for details).

> If I use sprintf instead, it works well. I was sure that ngx_sprintf
> was an alias to sprintf it's but not. Why are those kind of basic
> functions have been recoded ? Is there a reason not to use sprintf ?

There are at least two reasons:

1. Standard sprintf() uses null-terminated strings and unable to 
correctly work with binary data.  Also it wastes space and cpu 
time for terminating '\0' where it isn't needed.

2. Standard sprintf() implementations tends to behave badly on 
out-of-memory conditions, since they allocate internal buffers for 
various reasons and usually call abort() if allocation fails.

Maxim Dounin

> 
> >
> > Le 25 mars 2009 21:16, Jérôme Loyet <jerome at loyet.net> a écrit :
> >> 2009/3/25 Igor Sysoev <is at rambler-co.ru>:
> >>> On Tue, Mar 24, 2009 at 04:46:03PM +0100, J?r?me Loyet wrote:
> >>>
> >>>> I attach a patch which add the -p option to nginx. This option
> >>>> overrides the --prefix path and also the $NGX_CONF_PREFIX variable
> >>>> which is used as prefix for "include" directives. So, all relative
> >>>> path will be prefix with the value of this option.
> >>>>
> >>>> I don't know if it covers all cases but it seems ok in my env.
> >>>>
> >>>> Why did I want to make such a patch ?
> >>>> Because i'll be running several instances of nginx on the same box,
> >>>> with similar conf and similar directorys (/path/xx/conf,
> >>>> /path/xx/docs, /path/xx/logs, /path/xx/cache, /path/xx/tmp, ...). With
> >>>> same conf, I can launch multiple instances of nginx with
> >>>> /path/to/nginx -p /path/www, -p /patch/www2, ...
> >>>>
> >>>> I could run one instance of nginx with many "server" directives but
> >>>> the box is mutualized between clients and clients doesn't want to
> >>>> share their instance with others ...
> >>>>
> >>>> You could then imagine many cases in which this option could be usefull.
> >>>
> >>> I need to see how -p will affect on directives that use conf prefix
> >>> such as "include", "ssl_certficate*", and "auth_basic_user_file".
> >>> Historically, nginx had the single prefix (not conf one), but this was
> >>> uncomfortably for Linux packages, where now the conf prefix is /etc/nginx,
> >>> while default prefix is somewhere in other place.
> >>>
> >>
> >> I made a new version of my patch. The patch still add the -p prefix but:
> >> 1- in auto/options, I've set the following variables:
> >> +NGX_PREFIX_CONF_PATH=conf/nginx.conf
> >> +NGX_PREFIX_ERROR_LOG_PATH=logs/error.log
> >> +NGX_PREFIX_PID_PATH=logs/nginx.pid
> >> +NGX_PREFIX_LOCK_PATH=logs/nginx.lock
> >> +NGX_PREFIX_HTTP_CLIENT_TEMP_PATH=client_body_temp
> >> +NGX_PREFIX_HTTP_PROXY_TEMP_PATH=proxy_temp
> >> +NGX_PREFIX_HTTP_FASTCGI_TEMP_PATH=proxy_temp
> >> +NGX_PREFIX_HTTP_LOG_PATH=logs/access.log
> >>
> >> 2- in auto/options I use them in remplacement of RAW string, for exemple:
> >> -        NGX_CONF_PATH=$NGX_PREFIX/conf/nginx.conf
> >> +        NGX_CONF_PATH=$NGX_PREFIX/$NGX_PREFIX_CONF_PATH
> >>
> >> 3- 1) and 2) change nothing, except that default relative paths are in
> >> variable now
> >> 4- In configure, I export those variables
> >> 5- in each source file in which one of thoses defined variable are
> >> used, I changed it to
> >> 5a- if -p was not specified at runtime, nothing change. Use
> >> NGX_PID_PATH and NGX_PREFIX_PID_PATH is not used at all.
> >> 5b- if -p was specified at runtime, so don't use NGX_PID_PATH but "-p
> >> parameter"/NGX_PREFIX_PID_PATH
> >> 6- If the -p parameter doesn't finish with a '/' char, then add it (I
> >> didn't take care of windows path yet)
> >>
> >> I tried to make this patch clear and correct. It seams to work. But I
> >> didn't have time to test many features like ("ssl_certficate*", and
> >> "auth_basic_user_file). I'll try that tonight or tomorow.
> >>
> >> Hope this help.
> >>
> >> ++ Jerome
> >>
> >
>