Jdesk / memcached

Automatically exported from code.google.com/p/memcached
0 stars 0 forks source link

Ugly do_slabs_free #166

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
static void do_slabs_free(void *ptr, const size_t size, unsigned int id) {
...
    if (p->sl_curr == p->sl_total) { /* need more space on the free list */
        int new_size = (p->sl_total != 0) ? p->sl_total * 2 : 16;  /* 16 is arbitrary */
        void **new_slots = realloc(p->slots, new_size * sizeof(void *));
        if (new_slots == 0)
            return;
        p->slots = new_slots;
        p->sl_total = new_size;
    }

   According to this function, we know that, in order to maintain the free item, we should allocate a new memory. Actually it is unnecessary and while the realloc failed, do_slabs_free will cause memory leak.

What is the expected output? What do you see instead?
I fix the problem just as below. For the complete, please look at the 
attachment.

1.
/* powers-of-N allocation structures */
typedef struct {
    unsigned int size;      /* sizes of items */
    unsigned int perslab;   /* how many items per slab */
    item  *free_item_list;  /* a list of free item */
    void *end_page_ptr;         /* pointer to next free item at end of page, or 0 */
    unsigned int end_page_free; /* number of items remaining at end of last alloced page */

    unsigned int slabs;     /* how many slabs were allocated for this class */

    void **slab_list;       /* array of slab pointers */
    unsigned int list_size; /* size of prev array */

    unsigned int killing;  /* index+1 of dying slab, or zero if none */
    size_t requested; /* The number of requested bytes */
} slabclass_t;

2.
static void *do_slabs_alloc(const size_t size, unsigned int id) {

    /* fail unless we have space at the end of a recently allocated page,
       we have something on our freelist, or we could allocate a new page */
    if (! (p->end_page_ptr != 0 || p->free_item_list != NULL ||
           do_slabs_newslab(id) != 0)) {
        /* We don't have more memory available */
        ret = NULL;
    } else if (p->free_item_list != NULL) {
        /* return off our freelist */
        ret = p->free_item_list;
        p->free_item_list = p->free_item_list->next;
    } else {

3. do_slabs_free

static void do_slabs_free(item *ptr, const size_t size, unsigned int id) {
    slabclass_t *p;

    assert(ptr->slabs_clsid == 0);
    assert(id >= POWER_SMALLEST && id <= power_largest);
    if (id < POWER_SMALLEST || id > power_largest)
        return;

    MEMCACHED_SLABS_FREE(size, id, ptr);
    p = &slabclass[id];

#ifdef USE_SYSTEM_MALLOC
    mem_malloced -= size;
    free(ptr);
    return;
#endif

    ptr->next = p->free_item_list;
    p->free_item_list = ptr;
    p->requested -= size;
    return;
}

Original issue reported on code.google.com by zhuizhuhaomeng on 14 Nov 2010 at 3:22

Attachments:

GoogleCodeExporter commented 9 years ago
Sorry, I'm going to close this without trying the changes.

If you would like to get your change in, can you please resubmit with:

1) a unified diff (diff -u) of your change
2) the version your diff was generated against
3) ideally, some information on how to test the change

Thanks!

Original comment by dorma...@rydia.net on 8 Aug 2011 at 6:12