Skip to content

Commit bc939dd

Browse files
committed
mem: Fix cornercase bugs.
1 parent d9a6900 commit bc939dd

File tree

4 files changed

+84
-18
lines changed

4 files changed

+84
-18
lines changed

src/csnip/mem.c

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,30 @@
1111
#include <csnip/err.h>
1212
#include <csnip/mem.h>
1313

14-
void* csnip_mem_alloc(size_t n, size_t size)
14+
static inline size_t compute_alloc_amount(size_t n, size_t size)
1515
{
1616
if (size != 0 && SIZE_MAX / size < n) {
1717
/* Overflow */
18-
return NULL;
18+
return 0;
19+
}
20+
if ((size *= n) == 0) {
21+
/* malloc() with 0 size is not guaranteed to return a
22+
* non-NULL pointer, thus we make sure we allocate
23+
* always non-zero amounts.
24+
*
25+
* Also, we use zero to signal an error.
26+
*/
27+
size = 1;
1928
}
20-
return malloc(n * size);
29+
return size;
30+
}
31+
32+
void* csnip_mem_alloc(size_t n, size_t size)
33+
{
34+
size_t alloc_sz = compute_alloc_amount(n, size);
35+
if (alloc_sz == 0)
36+
return NULL;
37+
return malloc(alloc_sz);
2138
}
2239

2340
/* For aligned allocation, we use posix_memalign() if possible, since
@@ -31,22 +48,19 @@ void* csnip_mem_alloc(size_t n, size_t size)
3148

3249
void* csnip_mem_aligned_alloc(size_t nAlign, size_t n, size_t size, int* err_ret)
3350
{
34-
/* Compute the allocation size, taking care of possible overflow */
35-
if (size != 0 && SIZE_MAX / size < n) {
36-
if (err_ret)
37-
*err_ret = csnip_err_RANGE;
38-
return NULL;
39-
}
40-
size *= n;
51+
size_t alloc_sz = compute_alloc_amount(n, size);
4152

4253
#if defined(CSNIP_CONF__HAVE_POSIX_MEMALIGN) \
4354
|| !defined(CSNIP_CONF__HAVE_ALIGNED_ALLOC)
55+
if (nAlign < sizeof(void*))
56+
nAlign *= sizeof(void*);
57+
4458
void* p_ret;
4559
#ifdef CSNIP_CONF__HAVE_POSIX_MEMALIGN
46-
const int err = posix_memalign(&p_ret, nAlign, size);
60+
const int err = posix_memalign(&p_ret, nAlign, alloc_sz);
4761
#else
4862
int err = 0;
49-
p_ret = memalign(nAlign, size);
63+
p_ret = memalign(nAlign, alloc_sz);
5064
if (p_ret == NULL)
5165
err = errno;
5266
#endif
@@ -67,18 +81,18 @@ void* csnip_mem_aligned_alloc(size_t nAlign, size_t n, size_t size, int* err_ret
6781
return p_ret;
6882
#else
6983
/* use aligned_alloc() */
70-
const size_t rem = size % nAlign;
84+
const size_t rem = alloc_sz % nAlign;
7185
if (rem != 0) {
7286
const size_t toadd = nAlign - rem;
7387
/* Check for overflow */
74-
if (SIZE_MAX - toadd < size) {
88+
if (SIZE_MAX - toadd < alloc_sz) {
7589
if (err_ret)
7690
*err_ret = csnip_err_RANGE;
7791
return NULL;
7892
}
79-
size += toadd;
93+
alloc_sz += toadd;
8094
}
81-
void* p_ret = aligned_alloc(nAlign, size);
95+
void* p_ret = aligned_alloc(nAlign, alloc_sz);
8296
if (p_ret == NULL && err_ret != 0) {
8397
if (errno == ENOMEM) {
8498
*err_ret = csnip_err_NOMEM;

src/csnip/mem.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,9 @@ void csnip_mem_aligned_free(void* mem);
8181
/** Allocate an array with n entries and assign ptr to it.
8282
*
8383
* This is an expression version of mem_Alloc(), hence the -x
84-
* suffix. It assigns the pointer and returns an error value, or 0
85-
* in the success case. Generally more convenient to use than
84+
* suffix. It assigns the pointer and returns 0 in the success
85+
* case. In the error case, the pointer is set to NULL and an
86+
* error code is returned. Generally more convenient to use than
8687
* mem_Alloc.
8788
*/
8889
#define csnip_mem_Allocx(n, ptr) \

test/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ set(tests_c
1818
log_test1.c
1919
meanvar_test0.c
2020
mem_test0.c
21+
mem_test1.c
2122
mempool_test0.c
2223
ringbuf_test.c
2324
ringbuf2_test.c

test/mem_test1.c

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
#include <stdio.h>
2+
3+
#define CSNIP_SHORT_NAMES
4+
#include <csnip/mem.h>
5+
#include <csnip/util.h>
6+
7+
static int align_test()
8+
{
9+
int sizes[] = { 0, 1, 10, 100, 1000, 10000, 100000 };
10+
int aligns[] = { 1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024 };
11+
for (int i = 0; i < Static_len(sizes); ++i) {
12+
for (int j = 0; j < Static_len(aligns); ++j) {
13+
const int size = sizes[i];
14+
const int align = aligns[j];
15+
printf("Looking at size=%d, align=%d\n", size, align);
16+
17+
int err = 0;
18+
char* p;
19+
mem_AlignedAlloc(size, align, p, err);
20+
if (err != 0) {
21+
fprintf(stderr, "Alloc error: "
22+
"size=%d, align=%d: %d\n",
23+
sizes[i], aligns[j], err);
24+
return -1;
25+
}
26+
27+
Fill_n(p, sizes[i], 1);
28+
29+
int cnt = 0;
30+
for (int e = 0; e < size; ++e) {
31+
cnt += p[e];
32+
}
33+
if (cnt != size) {
34+
fprintf(stderr, "Verification failed!\n");
35+
return -1;
36+
}
37+
mem_AlignedFree(p);
38+
}
39+
}
40+
41+
return 0;
42+
}
43+
44+
int main(int argc, char** argv)
45+
{
46+
47+
if (align_test() != 0)
48+
return 1;
49+
return 0;
50+
}

0 commit comments

Comments
 (0)