diff --git a/cocci/isc_mempool_get_never_fail.spatch b/cocci/isc_mempool_get_never_fail.spatch new file mode 100644 index 0000000000..232ff9b369 --- /dev/null +++ b/cocci/isc_mempool_get_never_fail.spatch @@ -0,0 +1,41 @@ +@@ +statement S; +expression V; +@@ + +V = isc_mempool_get(...); +- if (V == NULL) S + +@@ +type T; +statement S; +expression V; +@@ + +V = (T *)isc_mempool_get(...); +- if (V == NULL) S + +@@ +statement S; +expression V; +@@ + +if (V == NULL) V = isc_mempool_get(...); +- if (V == NULL) S + +@@ +statement S1, S2; +expression V; +@@ + +V = isc_mempool_get(...); +- if (V == NULL) S1 else { S2 } ++ S2 + +@@ +type T; +expression V, E1, E2; +@@ + +- V = (T)isc_mempool_get(E1, E2); ++ V = isc_mempool_get(E1, E2); diff --git a/lib/dns/message.c b/lib/dns/message.c index ad21bb17b9..976f3e78ef 100644 --- a/lib/dns/message.c +++ b/lib/dns/message.c @@ -1076,10 +1076,6 @@ getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, goto cleanup; } rdataset = isc_mempool_get(msg->rdspool); - if (rdataset == NULL) { - result = ISC_R_NOMEMORY; - goto cleanup; - } /* * Convert rdatalist to rdataset, and attach the latter to @@ -1516,10 +1512,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, if (result == ISC_R_NOTFOUND) { rdataset = isc_mempool_get(msg->rdspool); - if (rdataset == NULL) { - result = ISC_R_NOMEMORY; - goto cleanup; - } free_rdataset = true; rdatalist = newrdatalist(msg); @@ -2558,9 +2550,6 @@ dns_message_gettemprdataset(dns_message_t *msg, dns_rdataset_t **item) { REQUIRE(item != NULL && *item == NULL); *item = isc_mempool_get(msg->rdspool); - if (*item == NULL) { - return (ISC_R_NOMEMORY); - } dns_rdataset_init(*item); return (ISC_R_SUCCESS); diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index f129285f90..fde46200a5 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -375,7 +375,6 @@ isc__mempool_create(isc_mem_t *mctx, size_t size, *\li mpctxp != NULL and *mpctxp == NULL * * Defaults: - *\li maxalloc = UINT_MAX *\li freemax = 1 *\li fillcount = 1 * @@ -412,9 +411,7 @@ isc_mempool_setname(isc_mempool_t *mpctx, const char *name); *unless the imposed externally provided locking protocols are followed. * * Also note that the quota limits will not always take immediate - *effect. For instance, setting "maxalloc" to a number smaller than the - *currently allocated count is permitted. New allocations will be - *refused until the count drops below this threshold. + * effect. * * All functions require (in addition to other requirements): * mpctx is a valid memory pool @@ -438,21 +435,6 @@ isc_mempool_getfreecount(isc_mempool_t *mpctx); * Returns current size of the free list. */ -unsigned int -isc_mempool_getmaxalloc(isc_mempool_t *mpctx); -/*!< - * Returns the maximum allowed number of allocations. - */ - -void -isc_mempool_setmaxalloc(isc_mempool_t *mpctx, unsigned int limit); -/*%< - * Sets the maximum allowed number of allocations. - * - * Additional requirements: - *\li limit > 0 - */ - unsigned int isc_mempool_getallocated(isc_mempool_t *mpctx); /*%< diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 9f3a6143d9..52e2b998ea 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -154,7 +154,6 @@ struct isc_mempool { ISC_LINK(isc_mempool_t) link; /*%< next pool in this mem context */ element *items; /*%< low water item list */ size_t size; /*%< size of each item on this pool */ - atomic_size_t maxalloc; /*%< max number of items allowed */ atomic_size_t allocated; /*%< # of items currently given out */ atomic_size_t freecount; /*%< # of items on reserved list */ atomic_size_t freemax; /*%< # of items allowed on free list */ @@ -805,15 +804,14 @@ isc_mem_stats(isc_mem_t *ctx, FILE *out) { pool = ISC_LIST_HEAD(ctx->pools); if (pool != NULL) { fprintf(out, "[Pool statistics]\n"); - fprintf(out, "%15s %10s %10s %10s %10s %10s %10s %10s %1s\n", - "name", "size", "maxalloc", "allocated", "freecount", - "freemax", "fillcount", "gets", "L"); + fprintf(out, "%15s %10s %10s %10s %10s %10s %10s %1s\n", "name", + "size", "allocated", "freecount", "freemax", + "fillcount", "gets", "L"); } while (pool != NULL) { fprintf(out, "%15s %10zu %10zu %10zu %10zu %10zu %10zu %10zu %s\n", - pool->name, pool->size, - atomic_load_relaxed(&pool->maxalloc), + pool->name, pool->size, (size_t)0, atomic_load_relaxed(&pool->allocated), atomic_load_relaxed(&pool->freecount), atomic_load_relaxed(&pool->freemax), @@ -1109,7 +1107,6 @@ isc__mempool_create(isc_mem_t *mctx, size_t size, .size = size, }; - atomic_init(&mpctx->maxalloc, SIZE_MAX); atomic_init(&mpctx->allocated, 0); atomic_init(&mpctx->freecount, 0); atomic_init(&mpctx->freemax, 1); @@ -1199,17 +1196,7 @@ void * isc__mempool_get(isc_mempool_t *mpctx FLARG) { REQUIRE(VALID_MEMPOOL(mpctx)); - allocated = atomic_fetch_add_release(&mpctx->allocated, 1); - maxalloc = atomic_load_acquire(&mpctx->maxalloc); - - /* - * Don't let the caller go over quota. - */ - if (ISC_UNLIKELY(allocated >= maxalloc)) { - atomic_fetch_sub_release(&mpctx->allocated, 1); - return (NULL); - } - + (void)atomic_fetch_add_relaxed(&mpctx->allocated, 1); atomic_fetch_add_relaxed(&mpctx->gets, 1); return (isc__mem_get(mpctx->mctx, mpctx->size FLARG_PASS)); @@ -1220,8 +1207,7 @@ isc__mempool_put(isc_mempool_t *mpctx, void *mem FLARG) { REQUIRE(VALID_MEMPOOL(mpctx)); REQUIRE(mem != NULL); - INSIST(atomic_fetch_sub_release(&mpctx->allocated, 1) > 0); - + atomic_fetch_sub_relaxed(&mpctx->allocated, 1); isc__mem_put(mpctx->mctx, mem, mpctx->size FLARG_PASS); } @@ -1234,16 +1220,7 @@ isc__mempool_get(isc_mempool_t *mpctx FLARG) { REQUIRE(VALID_MEMPOOL(mpctx)); - allocated = atomic_fetch_add_release(&mpctx->allocated, 1); - maxalloc = atomic_load_acquire(&mpctx->maxalloc); - - /* - * Don't let the caller go over quota - */ - if (ISC_UNLIKELY(allocated >= maxalloc)) { - atomic_fetch_sub_release(&mpctx->allocated, 1); - return (NULL); - } + (void)atomic_fetch_add_release(&mpctx->allocated, 1); if (ISC_UNLIKELY(mpctx->items == NULL)) { isc_mem_t *mctx = mpctx->mctx; @@ -1333,21 +1310,6 @@ isc_mempool_getfreecount(isc_mempool_t *mpctx) { return (atomic_load_relaxed(&mpctx->freecount)); } -void -isc_mempool_setmaxalloc(isc_mempool_t *mpctx, unsigned int limit) { - REQUIRE(VALID_MEMPOOL(mpctx)); - REQUIRE(limit > 0); - - atomic_store_release(&mpctx->maxalloc, limit); -} - -unsigned int -isc_mempool_getmaxalloc(isc_mempool_t *mpctx) { - REQUIRE(VALID_MEMPOOL(mpctx)); - - return (atomic_load_relaxed(&mpctx->maxalloc)); -} - unsigned int isc_mempool_getallocated(isc_mempool_t *mpctx) { REQUIRE(VALID_MEMPOOL(mpctx)); diff --git a/lib/isc/tests/mem_test.c b/lib/isc/tests/mem_test.c index d930971c73..c9602927f9 100644 --- a/lib/isc/tests/mem_test.c +++ b/lib/isc/tests/mem_test.c @@ -82,7 +82,6 @@ isc_mem_test(void **state) { isc_mempool_setfreemax(mp1, MP1_FREEMAX); isc_mempool_setfillcount(mp1, MP1_FILLCNT); - isc_mempool_setmaxalloc(mp1, MP1_MAXALLOC); /* * Allocate MP1_MAXALLOC items from the pool. This is our max. @@ -92,12 +91,6 @@ isc_mem_test(void **state) { assert_non_null(items1[i]); } - /* - * Try to allocate one more. This should fail. - */ - tmp = isc_mempool_get(mp1); - assert_null(tmp); - /* * Free the first 11 items. Verify that there are 10 free items on * the free list (which is our max).