[PATCH] Style: use ngx_str_set()

Maxim Dounin mdounin at mdounin.ru
Wed Aug 6 09:29:05 UTC 2014


On Tue, Aug 05, 2014 at 02:59:12PM -0700, Piotr Sikora wrote:

> Hey Maxim,
> > I don't like at least some of the changes, and I would rather say
> > no.
> Which one and why? I suspected that you might not like some of the
> changes, but I decided to send them all together instead of one by
> one... It doesn't have to be all or nothing.

Most notably I dislike changes in ngx_http_set_expires() and in 
ngx_http_auth_basic_user().  And some other changes as well, in 
particular the code in ngx_set_environment() looks strange when 
the var pointer (which is normally an ngx_str_t pointer used to 
iterate array) is used in ngx_str_set().

I don't think that changes of all the code to use ngx_str_set() is 
really something needed.  I rather think that both forms are 
acceptable, and one or another may be used whichever fits better.

> I also think that it's a good practice to set the struct (data+len)
> whenever possible, instead of operating on data and len fields
> separately, in different parts of code, for the sake of
> micro-optimization (i.e. auth_basic), especially when those fields are
> exported and can be accessed and modified by 3rd-party code.

In nginx, 3rd party code can access all the data, and there is 
more than one way to break things.

Maxim Dounin

More information about the nginx-devel mailing list