Commit d5e648e8 authored by Tim Rühsen's avatar Tim Rühsen 🛠

Make wget allocation functions overridable function pointers

* include/wget/wget.h: Remove allocation function prototypes,
  add function pointer types and 'extern' globals
* libwget/private.h: Use wget_free() instead of free()
* libwget/xalloc.c: Remove function definitions,
  add global function pointer variables
* src/options.c: Add own allocation functions
* unit-tests/test.c: Test that function pointers are overridable
parent 2a8986cf
Pipeline #66777296 passed with stages
in 44 minutes and 10 seconds
......@@ -416,25 +416,31 @@ WGETAPI void
WGETAPI int
wget_list_browse(const wget_list_t *list, wget_list_browse_t browse, void *context) G_GNUC_WGET_NONNULL((2));
/*
* Memory allocation routines
/**
* \ingroup libwget-xalloc
*
* Memory allocation function pointers
* @{
*/
// I try to never leave freed pointers hanging around
// Don't leave freed pointers hanging around
#define wget_xfree(a) do { if (a) { wget_free((void *)(a)); a=NULL; } } while (0)
typedef void *(*wget_oom_callback_t)(int size);
  • So, if I understand this correctly, we no longer have any OOM callback function.

    What happens when the user defines a malloc function, but:

    1. It returns a NULL. The library might de-reference a null-ptr and segfault
    2. It returns a smaller chunk of memory

    In general, we can assume that 2 never happens and if it does, it is a gross violation by the client. However, I don't think causing a segfault in the library due to 1 is a good idea. We still need to check for NULL and then propagate an error back to the caller. Maybe, we can have:

    # define _wget_malloc(size) do { \
      void *p = wget_malloc(size); \
      if (!p) return WGET_ERR_OOM; \
    } while(0);

    The syntax for the macro might be incorrect, but we can use such a macro throughout the library and change the functions prototypes where needed. WDYT?

    1. is not relevant (as you say)
    2. returning NULL is fine - I already started adding checks everywhere in the library code. In wget2 itself we don't need check since we catch the NULL case in our custom alloc functions and exit().
Please register or sign in to reply
WGETAPI void *
wget_malloc(size_t size) G_GNUC_WGET_MALLOC G_GNUC_WGET_ALLOC_SIZE(1);
WGETAPI void *
wget_calloc(size_t nmemb, size_t size) G_GNUC_WGET_MALLOC G_GNUC_WGET_ALLOC_SIZE2(1,2);
WGETAPI void *
wget_realloc(void *ptr, size_t size) G_GNUC_WGET_ALLOC_SIZE(2);
WGETAPI void
wget_free(void *ptr);
WGETAPI void
wget_set_oomfunc(wget_oom_callback_t);
/// Type of malloc() function
typedef void *(*wget_malloc_function) (size_t);
/// Type of calloc() function
typedef void *(*wget_calloc_function) (size_t, size_t);
/// Type of realloc() function
typedef void *(*wget_realloc_function) (void *, size_t);
/// Type of free() function
typedef void (*wget_free_function) (void *);
/* For use in callbacks */
extern WGETAPI wget_malloc_function wget_malloc;
  • Need to add documentation about these symbols and how the user can set them.

    Maybe we can even have the functions that we use in wget2 as an example for each of them

Please register or sign in to reply
extern WGETAPI wget_calloc_function wget_calloc;
extern WGETAPI wget_realloc_function wget_realloc;
extern WGETAPI wget_free_function wget_free;
/** @} */
/*
* String/Memory routines, slightly different than standard functions
......
......@@ -45,7 +45,7 @@
#endif
// I try to never leave freed pointers hanging around
#define xfree(a) do { if (a) { free((void *)(a)); a=NULL; } } while (0)
#define xfree(a) do { if (a) { wget_free((void *)(a)); a=NULL; } } while (0)
// number of elements within an array
#define countof(a) (sizeof(a)/sizeof(*(a)))
......
......@@ -38,137 +38,14 @@
* \defgroup libwget-xalloc Memory allocation functions
* @{
*
* The provided memory allocation functions are used by explicit libwget memory
* allocations.
* Global function pointers to memory allocation functions and to free().
*
* They differ from the standard ones in that they call an out-of-memory
* handler, if set by `wget_set_oomfunc()`. That OOM callback function of type
* `wget_oom_callback_t` will allow for a custom OOM handling.
*
* Some applications, like wget2, will simply print out a message an call exit().
* Others might want to do e.g. garbage collection and retry the allocation. Therefore
* the OOM callback has the size of the requested memory as argument and returns a void
* pointer (may be NULL) that will be returned by the libwget allocation function.
* If that function was `wget_calloc()`, the memory will be zeroed, if it was `wget_realloc()`,
* the old memory will be copied over and freed.
*/
static wget_oom_callback_t
_oom_callback;
static void *_no_memory(size_t size)
{
if (_oom_callback)
return _oom_callback(size);
return NULL;
}
/**
* \param[in] oom_callback Pointer to your custom out-of-memory function
*
* Set a custom out-of-memory function.
*
* If an out-of-memory condition occurs, the OOM callback function is called.
* If the OOM function returns 0, the allocation function is tried.
* Else the returned value is used to call exit().
*
* So you can set a OOM function to free temporarily allocated memory in ordre to
* continue operation.
*
*/
void wget_set_oomfunc(wget_oom_callback_t oom_callback)
{
_oom_callback = oom_callback;
}
/**
* \param[in] size Number of bytes to allocate
* \return A pointer to the allocated (uninitialized) memory
*
* Like the standard malloc(), except when the OOM callback function is set.
*
* If an out-of-memory condition occurs, the OOM callback function is called (if set).
* If the OOM function returns 0, wget_malloc() returns NULL. Else it exits with the returned
* exit status.
*/
void *wget_malloc(size_t size)
{
void *p = malloc(size);
if (!p)
p = _no_memory(size);
return p;
}
/**
* \param[in] nmemb Number of elements (each of size \p size) to allocate
* \param[in] size Size of element
* \return A pointer to the allocated (initialized) memory
*
* Like the standard calloc(), except when the OOM callback function is set.
*
* If an out-of-memory condition occurs the oom callback function is called (if set).
* If the OOM function returns 0, wget_calloc() returns NULL. Else it exits with the returned
* exit status.
* These pointers can be set to custom functions.
*/
void *wget_calloc(size_t nmemb, size_t size)
{
void *p = calloc(nmemb, size);
if (!p) {
p = _no_memory(nmemb * size);
if (p)
memset(p, 0, nmemb * size);
}
return p;
}
/**
* \param[in] ptr Pointer to old memory area
* \param[in] size Number of bytes to allocate for the new memory area
* \return A pointer to the new memory area
*
* Like the standard realloc(), except when the OOM callback function is set.
*
* If an out-of-memory condition occurs the OOM callback function is called (if set).
* If the OOM function returns 0, wget_realloc() returns NULL. Else it exits with the returned
* exit status.
*/
void *wget_realloc(void *ptr, size_t size)
{
void *p;
if (!(p = realloc(ptr, size))) {
if (!size && ptr)
return NULL; // realloc() acted as free().
p = _no_memory(size);
if (p) {
memmove(p, ptr, size);
free(ptr);
}
}
return p;
}
/**
* \param[in] ptr Pointer to memory-pointer to be freed
*
* This function is like free().
*
* It is basically needed on systems where the library malloc heap is different
* from the caller's malloc heap, which happens on Windows when the library
* is a separate DLL.
*
* To prevent typical use-after-free issues, use the macro wget_xfree().
*/
void wget_free(void *ptr)
{
free(ptr);
}
wget_malloc_function wget_malloc = malloc;
wget_calloc_function wget_calloc = calloc;
wget_realloc_function wget_realloc = realloc;
wget_free_function wget_free = free;
  • Can we have these definitions in include/wget/wget.h? It allows the user to see the defaults easily.

  • No. Then each object file would have it's own version of those variables.

Please register or sign in to reply
/**@}*/
......@@ -64,6 +64,8 @@
# include "wget_gpgme.h"
#endif
static void set_allocation_functions(void);
static exit_status_t
exit_status;
......@@ -2744,13 +2746,6 @@ static int G_GNUC_WGET_NONNULL((2)) parse_command_line(int argc, const char **ar
return n;
}
static void _no_memory(void)
{
fputs("No memory\n", stderr);
exit(EXIT_FAILURE);
}
// Return the user's home directory (strdup-ed), or NULL if none is found.
static const char *get_home_dir(bool free_home)
{
......@@ -3140,8 +3135,8 @@ int init(int argc, const char **argv)
{
int n, rc;
// set libwget out-of-memory function
wget_set_oomfunc((wget_oom_callback_t) _no_memory);
// use our own allocation functions so we can print error + exit() in a out-of-memory situation
set_allocation_functions();
// this is a special case for switching on debugging before any config file is read
if (argc >= 2) {
......@@ -4042,3 +4037,53 @@ int selftest_options(void)
return ret;
}
G_GNUC_WGET_NORETURN
static void no_memory(void)
{
fputs("No memory\n", stderr);
exit(EXIT_FAILURE);
}
static void *my_malloc(size_t size)
{
void *p = malloc(size);
if (p)
return p;
no_memory();
}
static void *my_calloc(size_t nmemb, size_t size)
{
void *p = calloc(nmemb, size);
if (p)
return p;
no_memory();
}
static void *my_realloc(void *ptr, size_t size)
{
void *p = realloc(ptr, size);
if (p || (ptr && size == 0))
return p;
no_memory();
}
static void my_free(void *ptr)
{
free(ptr);
}
static void set_allocation_functions(void)
{
wget_malloc = my_malloc;
wget_calloc = my_calloc;
wget_realloc = my_realloc;
wget_free = my_free;
}
......@@ -2425,6 +2425,32 @@ static void test_parse_response_header(void)
xfree(response_text);
}
static unsigned alloc_flags;
static void *test_malloc(size_t size)
{
alloc_flags |= 1;
return malloc(size);
}
static void *test_calloc(size_t nmemb, size_t size)
{
alloc_flags |= 2;
return calloc(nmemb, size);
}
static void *test_realloc(void *ptr, size_t size)
{
alloc_flags |= 4;
return realloc(ptr, size);
}
static void test_free(void *ptr)
{
alloc_flags |= 8;
free(ptr);
}
int main(int argc, const char **argv)
{
// if VALGRIND testing is enabled, we have to call ourselves with valgrind checking
......@@ -2449,7 +2475,10 @@ int main(int argc, const char **argv)
return -1;
srand((unsigned int) time(NULL));
wget_set_oomfunc((wget_oom_callback_t) abort);
wget_malloc = test_malloc;
wget_calloc = test_calloc;
wget_realloc = test_realloc;
wget_free = test_free;
// testing basic library functionality
test_mem();
......@@ -2491,6 +2520,13 @@ int main(int argc, const char **argv)
deinit(); // free resources allocated by init()
if (alloc_flags == 0xF)
ok++;
else {
failed++;
info_printf("alloc_flags %X\n", alloc_flags);
}
if (failed) {
info_printf("Summary: %d out of %d tests failed\n", failed, ok + failed);
return 1;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment