From 095d89995e49ef96ac0d82793702acce95b71459 Mon Sep 17 00:00:00 2001 From: hniksic Date: Sun, 2 Nov 2003 13:12:49 -0800 Subject: [PATCH] [svn] Abort on xfree(NULL). --- src/ChangeLog | 14 ++++++++++++++ src/html-url.c | 13 +++++++++---- src/init.c | 3 ++- src/wget.h | 5 ----- src/xmalloc.c | 40 ++++++++++++++++++++++++++++++++++++---- src/xmalloc.h | 17 +++++++++++++---- 6 files changed, 74 insertions(+), 18 deletions(-) diff --git a/src/ChangeLog b/src/ChangeLog index 62b68db3..9622f283 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,17 @@ +2003-11-02 Hrvoje Niksic + + * html-url.c (cleanup_html_url): Destroy the hash tables, don't + just call free on them. + (init_interesting): Use hash_table_put instead of string_set_add + because we don't need the strdup that the latter function + performs. + + * init.c (cleanup): Don't pass NULL to cookie_jar_delete. + + * xmalloc.c (xfree_real): Abort when passed a NULL pointer. + (xfree_debug): Print at the file and line of the offending call to + free. + 2003-11-02 Hrvoje Niksic * wget.h: Retired the `boolean' type. Moved the DEFAULT_LOGFILE diff --git a/src/html-url.c b/src/html-url.c index efbae629..54dbf382 100644 --- a/src/html-url.c +++ b/src/html-url.c @@ -227,9 +227,10 @@ init_interesting (void) /* Add the attributes we care about. */ interesting_attributes = make_nocase_string_hash_table (10); for (i = 0; i < countof (additional_attributes); i++) - string_set_add (interesting_attributes, additional_attributes[i]); + hash_table_put (interesting_attributes, additional_attributes[i], "1"); for (i = 0; i < countof (tag_url_attributes); i++) - string_set_add (interesting_attributes, tag_url_attributes[i].attr_name); + hash_table_put (interesting_attributes, + tag_url_attributes[i].attr_name, "1"); } /* Find the value of attribute named NAME in the taginfo TAG. If the @@ -715,6 +716,10 @@ get_urls_file (const char *file) void cleanup_html_url (void) { - xfree_null (interesting_tags); - xfree_null (interesting_attributes); + /* Destroy the hash tables. The hash table keys and values are not + allocated by this code, so we don't need to free them here. */ + if (interesting_tags) + hash_table_destroy (interesting_tags); + if (interesting_attributes) + hash_table_destroy (interesting_attributes); } diff --git a/src/init.c b/src/init.c index 7f666f9e..391b32b9 100644 --- a/src/init.c +++ b/src/init.c @@ -1319,7 +1319,8 @@ cleanup (void) cleanup_html_url (); downloaded_files_free (); host_cleanup (); - cookie_jar_delete (wget_cookie_jar); + if (wget_cookie_jar) + cookie_jar_delete (wget_cookie_jar); { extern acc_t *netrc_list; diff --git a/src/wget.h b/src/wget.h index b75aab60..9cac5c75 100644 --- a/src/wget.h +++ b/src/wget.h @@ -40,11 +40,6 @@ so, delete this exception statement from your version. */ # define NDEBUG #endif -/* Define this if you want primitive but extensive malloc debugging. - It will make Wget extremely slow, so only do it in development - builds. */ -#undef DEBUG_MALLOC - #ifndef PARAMS # if PROTOTYPES # define PARAMS(args) args diff --git a/src/xmalloc.c b/src/xmalloc.c index 2f2a9006..311506ec 100644 --- a/src/xmalloc.c +++ b/src/xmalloc.c @@ -83,7 +83,7 @@ memfatal (const char *context, long attempted_size) #define xmalloc0 xmalloc0_real #define xrealloc xrealloc_real #define xstrdup xstrdup_real - #define xfree free + #define xfree xfree_real In case of memory debugging, the definitions are a bit more complex, because we want to provide more information, *and* we want @@ -93,9 +93,9 @@ memfatal (const char *context, long attempted_size) #define xmalloc(a) xmalloc_debug (a, __FILE__, __LINE__) #define xmalloc0(a) xmalloc0_debug (a, __FILE__, __LINE__) - #define xfree(a) xfree_debug (a, __FILE__, __LINE__) #define xrealloc(a, b) xrealloc_debug (a, b, __FILE__, __LINE__) #define xstrdup(a) xstrdup_debug (a, __FILE__, __LINE__) + #define xfree(a) xfree_debug (a, __FILE__, __LINE__) Each of the *_debug function does its magic and calls the real one. */ @@ -163,6 +163,30 @@ xstrdup_real (const char *s) return copy; } +STATIC_IF_DEBUG void +xfree_real (void *ptr) +{ + /* Wget's xfree() must not be passed a NULL pointer. This is for + historical reasons: many pre-C89 systems were known to bomb at + free(NULL), and Wget was careful to use xfree_null when there is + a possibility of PTR being NULL. (It might have been better to + simply have xfree() do nothing if ptr==NULL.) + + Since the code is already written that way, this assert simply + enforces that constraint. Code that thinks it doesn't deal with + NULL, and it in fact does, aborts immediately. If you trip on + this, either the code has a pointer handling bug or should have + called xfree_null instead of xfree. Correctly written code + should never trigger this assertion. + + If the assertion proves to be too much of a hassle, it can be + removed and a check that makes NULL a no-op placed in its stead. + If that is done, xfree_null is no longer needed and should be + removed. */ + assert (ptr != NULL); + free (ptr); +} + /* xfree_real is unnecessary because free doesn't require any special functionality. */ @@ -300,10 +324,18 @@ xstrdup_debug (const char *s, const char *source_file, int source_line) void xfree_debug (void *ptr, const char *source_file, int source_line) { - assert (ptr != NULL); + /* See xfree_real for rationale of this abort. We repeat it here + because we can print the file and the line where the offending + free occurred. */ + if (ptr == NULL) + { + fprintf ("%s: xfree(NULL) at %s:%d\n", + exec_name, source_file, source_line); + abort (); + } ++free_count; unregister_ptr (ptr); - free (ptr); + xfree_real (ptr); } #endif /* DEBUG_MALLOC */ diff --git a/src/xmalloc.h b/src/xmalloc.h index d738afc5..7729e5ab 100644 --- a/src/xmalloc.h +++ b/src/xmalloc.h @@ -30,10 +30,18 @@ so, delete this exception statement from your version. */ #ifndef XMALLOC_H #define XMALLOC_H +/* Define this if you want primitive but extensive malloc debugging. + It will make Wget extremely slow, so only do it in development + builds. */ +#define DEBUG_MALLOC + /* When DEBUG_MALLOC is not defined (which is normally the case), the allocation functions directly map to *_real wrappers. In the DEBUG_MALLOC mode, they also record the file and line where the - offending malloc/free/... was invoked. */ + offending malloc/free/... was invoked. + + *Note*: xfree(NULL) aborts. If the pointer you're freeing can be + NULL, use xfree_null instead. */ #ifndef DEBUG_MALLOC @@ -41,26 +49,27 @@ so, delete this exception statement from your version. */ #define xmalloc0 xmalloc0_real #define xrealloc xrealloc_real #define xstrdup xstrdup_real -#define xfree free +#define xfree xfree_real void *xmalloc_real PARAMS ((size_t)); void *xmalloc0_real PARAMS ((size_t)); void *xrealloc_real PARAMS ((void *, size_t)); char *xstrdup_real PARAMS ((const char *)); +void xfree_real PARAMS ((void *)); #else /* DEBUG_MALLOC */ #define xmalloc(s) xmalloc_debug (s, __FILE__, __LINE__) #define xmalloc0(s) xmalloc0_debug (s, __FILE__, __LINE__) -#define xfree(p) xfree_debug (p, __FILE__, __LINE__) #define xrealloc(p, s) xrealloc_debug (p, s, __FILE__, __LINE__) #define xstrdup(p) xstrdup_debug (p, __FILE__, __LINE__) +#define xfree(p) xfree_debug (p, __FILE__, __LINE__) void *xmalloc_debug PARAMS ((size_t, const char *, int)); void *xmalloc0_debug PARAMS ((size_t, const char *, int)); -void xfree_debug PARAMS ((void *, const char *, int)); void *xrealloc_debug PARAMS ((void *, size_t, const char *, int)); char *xstrdup_debug PARAMS ((const char *, const char *, int)); +void xfree_debug PARAMS ((void *, const char *, int)); #endif /* DEBUG_MALLOC */