One line redundant code in ngx_slab_free_locked

[chronolaw@…]

Hi, When I read the source code of Nginx, I notice there is one line redundant code in ngx_slab_free_locked.

n = ((u_char *) p - pool->start) >> ngx_pagesize_shift;

In the beginning of this function, 'n' has been calculated, but in the case NGX_SLAB_PAGE, it is calculated again.

Please see the patch, thanks.

[Maxim Dounin]

The n variable is used in the following &pool->pages[n] reference, and using generic multi-purpose variables in the parts of code distant from the code where it is calculated is generally a bad idea. Even if in a particular moment the variable stays intact, it can be overwritten at some point later, breaking things.

On the other hand, calculating &pool->pages[n] looks unneeded too, as we already have page pointer. It should be fine to simply use page instead of re-calculating it. Please test if the following patch works for you:

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1549854991 -10800
#      Mon Feb 11 06:16:31 2019 +0300
# Node ID aa5fa9166b0b81f084f64b578707ec807d03cab1
# Parent  1aa074869d804fe822add52dbaeda978fc749cf6
Slab: removed redundant page calculation (ticket #1721).

diff --git a/src/core/ngx_slab.c b/src/core/ngx_slab.c
--- a/src/core/ngx_slab.c
+++ b/src/core/ngx_slab.c
@@ -635,10 +635,9 @@ ngx_slab_free_locked(ngx_slab_pool_t *po
             goto fail;
         }
 
-        n = ((u_char *) p - pool->start) >> ngx_pagesize_shift;
         size = slab & ~NGX_SLAB_PAGE_START;
 
-        ngx_slab_free_pages(pool, &pool->pages[n], size);
+        ngx_slab_free_pages(pool, page, size);
 
         ngx_slab_junk(p, size << ngx_pagesize_shift);
 

[chronolaw@…]

I think this is a better patch, thanks.

[Maxim Dounin <mdounin@…>]

In 7457:d97d09ef3afe/nginx:

Slab: removed redundant page calculation (ticket #1721).

[Maxim Dounin]

Committed, thanks.