Skip to content

Commit d02f6cb

Browse files
Fix crash in nk_buffer_realloc and forbid the use of realloc
In short: the default implementation of nk_allocator (aka nk_malloc) is fundamentally broken, and should have used realloc instead of malloc. This led to the strange workaround in nk_buffer_realloc that would crash whenever you use custom allocator with correct realloc assumption. We cannot change nk_malloc at this point, as people could have implemented their own allocators based on it, but we can ensure that Nuklear's internal code will never try to reallocate memory. Fixes: #768 Reported-by: David Delassus <david.jose.delassus@gmail.com>
1 parent a329721 commit d02f6cb

File tree

3 files changed

+34
-25
lines changed

3 files changed

+34
-25
lines changed

demo/sdl3_renderer/nuklear_sdl3_renderer.h

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -112,16 +112,7 @@ NK_INTERN void *
112112
nk_sdl_alloc(nk_handle user, void *old, nk_size size)
113113
{
114114
NK_UNUSED(user);
115-
/* FIXME: nk_sdl_alloc should use SDL_realloc here, not SDL_malloc
116-
* but this could cause a double-free due to bug within Nuklear, see:
117-
* https://github.com/Immediate-Mode-UI/Nuklear/issues/768
118-
* */
119-
#if 0
120115
return SDL_realloc(old, size);
121-
#else
122-
NK_UNUSED(old);
123-
return SDL_malloc(size);
124-
#endif
125116
}
126117

127118
NK_INTERN void

nuklear.h

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8524,7 +8524,16 @@ NK_LIB void*
85248524
nk_malloc(nk_handle unused, void *old,nk_size size)
85258525
{
85268526
NK_UNUSED(unused);
8527-
NK_UNUSED(old);
8527+
/*
8528+
* Due to historical reasons, Nuklear always calls alloc(user,old,size)
8529+
* with the "old" pointer always being NULL. For full explanation, see:
8530+
* https://github.com/Immediate-Mode-UI/Nuklear/issues/768
8531+
*
8532+
* FIXME: The best would be to replace this function with "nk_realloc",
8533+
* but some custom allocators could still depend on "malloc" assumption
8534+
* so changing this now (without major bump) would break existing code.
8535+
*/
8536+
NK_ASSERT(!old && "Nuklear's internal code must never call realloc !");
85288537
return malloc(size);
85298538
}
85308539
NK_LIB void
@@ -8617,15 +8626,15 @@ nk_buffer_realloc(struct nk_buffer *b, nk_size capacity, nk_size *size)
86178626
return 0;
86188627

86198628
buffer_size = b->memory.size;
8620-
temp = b->pool.alloc(b->pool.userdata, b->memory.ptr, capacity);
8621-
NK_ASSERT(temp);
8622-
if (!temp) return 0;
8629+
NK_ASSERT(capacity >= buffer_size && "shrinking was never supported here");
86238630

8631+
/* HACK: this simulates realloc with malloc+memcpy+free
8632+
* for backwards compatibility reasons, see the note in nk_malloc */
8633+
temp = b->pool.alloc(b->pool.userdata, 0, capacity);
8634+
if (!temp) return 0;
86248635
*size = capacity;
8625-
if (temp != b->memory.ptr) {
8626-
NK_MEMCPY(temp, b->memory.ptr, buffer_size);
8627-
b->pool.free(b->pool.userdata, b->memory.ptr);
8628-
}
8636+
NK_MEMCPY(temp, b->memory.ptr, buffer_size);
8637+
b->pool.free(b->pool.userdata, b->memory.ptr);
86298638

86308639
if (b->size == buffer_size) {
86318640
/* no back buffer so just set correct size */

src/nuklear_buffer.c

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,16 @@ NK_LIB void*
1111
nk_malloc(nk_handle unused, void *old,nk_size size)
1212
{
1313
NK_UNUSED(unused);
14-
NK_UNUSED(old);
14+
/*
15+
* Due to historical reasons, Nuklear always calls alloc(user,old,size)
16+
* with the "old" pointer always being NULL. For full explanation, see:
17+
* https://github.com/Immediate-Mode-UI/Nuklear/issues/768
18+
*
19+
* FIXME: The best would be to replace this function with "nk_realloc",
20+
* but some custom allocators could still depend on "malloc" assumption
21+
* so changing this now (without major bump) would break existing code.
22+
*/
23+
NK_ASSERT(!old && "Nuklear's internal code must never call realloc !");
1524
return malloc(size);
1625
}
1726
NK_LIB void
@@ -104,15 +113,15 @@ nk_buffer_realloc(struct nk_buffer *b, nk_size capacity, nk_size *size)
104113
return 0;
105114

106115
buffer_size = b->memory.size;
107-
temp = b->pool.alloc(b->pool.userdata, b->memory.ptr, capacity);
108-
NK_ASSERT(temp);
109-
if (!temp) return 0;
116+
NK_ASSERT(capacity >= buffer_size && "shrinking was never supported here");
110117

118+
/* HACK: this simulates realloc with malloc+memcpy+free
119+
* for backwards compatibility reasons, see the note in nk_malloc */
120+
temp = b->pool.alloc(b->pool.userdata, 0, capacity);
121+
if (!temp) return 0;
111122
*size = capacity;
112-
if (temp != b->memory.ptr) {
113-
NK_MEMCPY(temp, b->memory.ptr, buffer_size);
114-
b->pool.free(b->pool.userdata, b->memory.ptr);
115-
}
123+
NK_MEMCPY(temp, b->memory.ptr, buffer_size);
124+
b->pool.free(b->pool.userdata, b->memory.ptr);
116125

117126
if (b->size == buffer_size) {
118127
/* no back buffer so just set correct size */

0 commit comments

Comments
 (0)