Welcome! Log In Create A New Profile

Advanced

Re: [Patch] add -p option to override prefix

All files from this thread

File Name File Size   Posted by Date  
nginx-prefix.patch 1.9 KB open | download Jérôme Loyet 03/24/2009 Read message
ngx_runtime_config.patch 2 KB open | download Manlio Perillo 03/25/2009 Read message
nginx-prefix-v2.patch 14.4 KB open | download Jérôme Loyet 03/25/2009 Read message
nginx-prefix-full-v2.patch 15.8 KB open | download Jérôme Loyet 03/25/2009 Read message
nginx-prefix-v2.patch 18 KB open | download Jérôme Loyet 03/31/2009 Read message
Maxim Dounin
March 26, 2009 08:56AM
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@loyet.net> a écrit :
> >> 2009/3/25 Igor Sysoev <is@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
> >>
> >
>
Subject Author Posted

[Patch] add -p option to override prefix Attachments

Jérôme Loyet March 24, 2009 11:46AM

Re: [Patch] add -p option to override prefix

Juan Fco. Giordana March 24, 2009 12:27PM

Re: [Patch] add -p option to override prefix

Jérôme Loyet March 24, 2009 01:17PM

Re: [Patch] add -p option to override prefix

Kirill A. Korinskiy March 24, 2009 03:33PM

Re: [Patch] add -p option to override prefix

Jérôme Loyet March 24, 2009 05:59PM

Re: [Patch] add -p option to override prefix

Igor Sysoev March 25, 2009 11:56AM

Re: [Patch] add -p option to override prefix Attachments

Jérôme Loyet March 25, 2009 04:16PM

Re: [Patch] add -p option to override prefix Attachments

Jérôme Loyet March 25, 2009 05:55PM

Re: [Patch] add -p option to override prefix

Jérôme Loyet March 26, 2009 08:27AM

Re: [Patch] add -p option to override prefix

Maxim Dounin March 26, 2009 08:56AM

Re: [Patch] add -p option to override prefix

Igor Sysoev March 26, 2009 09:02AM

Re: [Patch] add -p option to override prefix Attachments

Jérôme Loyet March 31, 2009 04:34PM

Re: [Patch] add -p option to override prefix Attachments

Manlio Perillo March 25, 2009 03:32PM



Sorry, only registered users may post in this forum.

Click here to login

Online Users

Guests: 235
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready