From c59bf4bbadee6c39413567475515600af53eba0a Mon Sep 17 00:00:00 2001 From: Reuben Thomas Date: Fri, 26 Jan 2018 11:49:06 +0000 Subject: [PATCH] Fix memory leaks Use the existing, but unused, term_routine member of struct recode_step to register finalisers. Also add step_table_term_routine, as adding separate finalisers for different table types is nicer than having to add finalisers for a huge number of transformers. Zero-initialise all allocated memory, so we can assume it, and as a defensive measure. Move struct ucs2_to_byte from recodext.h into recode.c, its only user. --- src/cdcnos.c | 1 + src/combine.c | 8 +++++--- src/ebcdic.c | 3 +++ src/html.c | 2 ++ src/iconv.c | 2 ++ src/lat1ansel.c | 1 + src/lat1asci.c | 1 + src/lat1btex.c | 1 + src/lat1iso5426.c | 1 + src/lat1ltex.c | 1 + src/lat1txte.c | 1 + src/recode.c | 38 +++++++++++++++++++++++++++++++++++--- src/recodext.h | 9 ++------- src/request.c | 43 +++++++++++++++++++++++++++++++++++++++++-- src/rfc1345.c | 8 ++++++++ 15 files changed, 105 insertions(+), 15 deletions(-) diff --git a/src/cdcnos.c b/src/cdcnos.c index b18e5f8..c62ec25 100644 --- a/src/cdcnos.c +++ b/src/cdcnos.c @@ -174,6 +174,7 @@ init_ascii_cdcnos (RECODE_STEP step, step->step_type = RECODE_BYTE_TO_STRING; step->step_table = table; + step->step_table_term_routine = free; return true; } diff --git a/src/combine.c b/src/combine.c index 7b30745..be1243d 100644 --- a/src/combine.c +++ b/src/combine.c @@ -20,7 +20,7 @@ #include "common.h" #include "hash.h" -/* FIXME: Cleanup memory at end of job, and softly report errors. */ +/* FIXME: Softly report errors. */ /* The satisfactory aspects are that Recode is now able to combine a set of sequence of UCS-2 characters into single codes, or explode those single @@ -97,6 +97,7 @@ init_explode (RECODE_STEP step, return false; step->step_type = RECODE_EXPLODE_STEP; step->step_table = table; + step->step_table_term_routine = (void (*) (void *)) hash_free; if (!data) return true; @@ -285,12 +286,12 @@ static void state_free (void *void_state) { struct state *state = (struct state *) void_state; - struct state *shift = (struct state *) state->shift; + struct state *shift = state->shift; while (shift != NULL) { struct state *next_shift = shift->next; - free (shift); + state_free (shift); shift = next_shift; } free (state); @@ -390,6 +391,7 @@ init_combine (RECODE_STEP step, return false; step->step_type = RECODE_COMBINE_STEP; step->step_table = table; + step->step_table_term_routine = (void (*) (void *)) hash_free; if (!data) return true; diff --git a/src/ebcdic.c b/src/ebcdic.c index 75dbbc1..4428f9f 100644 --- a/src/ebcdic.c +++ b/src/ebcdic.c @@ -164,6 +164,7 @@ init_ebcdic_ascii (RECODE_STEP step, step->step_table) { step->step_type = RECODE_BYTE_TO_BYTE; + step->step_table_term_routine = free; return true; } else @@ -199,6 +200,7 @@ init_ebcdic_ccc_ascii (RECODE_STEP step, step->step_table) { step->step_type = RECODE_BYTE_TO_BYTE; + step->step_table_term_routine = free; return true; } else @@ -233,6 +235,7 @@ init_ebcdic_ibm_ascii (RECODE_STEP step, step->step_table) { step->step_type = RECODE_BYTE_TO_BYTE; + step->step_table_term_routine = free; return true; } else diff --git a/src/html.c b/src/html.c index 162c6a3..09b9d93 100644 --- a/src/html.c +++ b/src/html.c @@ -443,6 +443,7 @@ init_ucs2_html (RECODE_STEP step, step->step_type = RECODE_UCS2_TO_STRING; step->step_table = table; + step->step_table_term_routine = (void (*) (void *)) hash_free; return true; } @@ -627,6 +628,7 @@ init_html_ucs2 (RECODE_STEP step, step->step_type = RECODE_STRING_TO_UCS2; step->step_table = table; + step->step_table_term_routine = (void (*) (void *)) hash_free; return true; } diff --git a/src/iconv.c b/src/iconv.c index c1b7a06..d37999f 100644 --- a/src/iconv.c +++ b/src/iconv.c @@ -129,6 +129,8 @@ wrapped_transform (iconv_t conversion, RECODE_SUBTASK subtask) free (check_output_buffer); } + + iconv_close (check_conversion); } /* Invalid or untranslatable input. Skip one byte. */ diff --git a/src/lat1ansel.c b/src/lat1ansel.c index e14f599..5cb6675 100644 --- a/src/lat1ansel.c +++ b/src/lat1ansel.c @@ -73,6 +73,7 @@ init_latin1_ansel (RECODE_STEP step, table[cursor->code] = cursor->string; step->step_table = table; + step->step_table_term_routine = free; return true; } diff --git a/src/lat1asci.c b/src/lat1asci.c index 736ee39..c5d9757 100644 --- a/src/lat1asci.c +++ b/src/lat1asci.c @@ -183,6 +183,7 @@ init_latin1_ascii (RECODE_STEP step, step->step_type = RECODE_BYTE_TO_STRING; step->step_table = table; + step->step_table_term_routine = free; return true; } diff --git a/src/lat1btex.c b/src/lat1btex.c index 1d2100b..f27284b 100644 --- a/src/lat1btex.c +++ b/src/lat1btex.c @@ -162,6 +162,7 @@ init_latin1_bibtex (RECODE_STEP step, step->step_type = RECODE_BYTE_TO_STRING; step->step_table = table; + step->step_table_term_routine = free; return true; } diff --git a/src/lat1iso5426.c b/src/lat1iso5426.c index fa50a3a..2a186ba 100644 --- a/src/lat1iso5426.c +++ b/src/lat1iso5426.c @@ -71,6 +71,7 @@ init_latin1_iso5426 (RECODE_STEP step, table[cursor->code] = cursor->string; step->step_table = table; + step->step_table_term_routine = free; return true; } diff --git a/src/lat1ltex.c b/src/lat1ltex.c index 43e9898..8fa8959 100644 --- a/src/lat1ltex.c +++ b/src/lat1ltex.c @@ -164,6 +164,7 @@ init_latin1_latex (RECODE_STEP step, step->step_type = RECODE_BYTE_TO_STRING; step->step_table = table; + step->step_table_term_routine = free; return true; } diff --git a/src/lat1txte.c b/src/lat1txte.c index 5aade2c..df4d5da 100644 --- a/src/lat1txte.c +++ b/src/lat1txte.c @@ -210,6 +210,7 @@ init_latin1_texte (RECODE_STEP step, step->step_type = RECODE_BYTE_TO_STRING; step->step_table = table; + step->step_table_term_routine = free; return true; } diff --git a/src/recode.c b/src/recode.c index 7372286..89234af 100644 --- a/src/recode.c +++ b/src/recode.c @@ -63,7 +63,7 @@ recode_malloc (RECODE_OUTER outer, size_t size) { void *result; - result = malloc (size); + result = calloc (1, size); if (!result) recode_error (outer, _("Virtual memory exhausted")); @@ -248,6 +248,7 @@ complete_pairs (RECODE_OUTER outer, RECODE_STEP step, memcpy (table, reverse ? right_table : left_table, 256); step->step_type = RECODE_BYTE_TO_BYTE; step->step_table = table; + step->step_table_term_routine = free; /* Upgrade step quality to reversible. */ @@ -299,6 +300,7 @@ complete_pairs (RECODE_OUTER outer, RECODE_STEP step, step->transform_routine = transform_byte_to_variable; step->step_type = RECODE_BYTE_TO_STRING; step->step_table = table2; + step->step_table_term_routine = free; } return true; @@ -343,6 +345,18 @@ transform_byte_to_ucs2 (RECODE_SUBTASK subtask) | Recode a file from double byte UCS-2 characters to one byte characters. | `-------------------------------------------------------------------------*/ +struct ucs2_to_byte + { + recode_ucs2 code; /* UCS-2 value */ + unsigned char byte; /* corresponding byte */ + }; + +struct ucs2_to_byte_local + { + Hash_table *table; + struct ucs2_to_byte *data; + }; + static size_t ucs2_to_byte_hash (const void *void_data, size_t table_size) { @@ -360,6 +374,15 @@ ucs2_to_byte_compare (const void *void_first, const void *void_second) return first->code == second->code; } +static bool +term_ucs2_to_byte (RECODE_STEP step) +{ + hash_free (((struct ucs2_to_byte_local *) step->local)->table); + free (((struct ucs2_to_byte_local *) step->local)->data); + free (step->local); + return true; +} + bool init_ucs2_to_byte (RECODE_STEP step, RECODE_CONST_REQUEST request, @@ -397,14 +420,23 @@ init_ucs2_to_byte (RECODE_STEP step, } } - step->local = table; + if (!ALLOC (step->local, 1, struct ucs2_to_byte_local)) + { + hash_free (table); + free (data); + return false; + } + ((struct ucs2_to_byte_local *) step->local)->table = table; + ((struct ucs2_to_byte_local *) step->local)->data = data; + + step->term_routine = term_ucs2_to_byte; return true; } bool transform_ucs2_to_byte (RECODE_SUBTASK subtask) { - Hash_table *table = (Hash_table *) subtask->step->local; + Hash_table *table = ((struct ucs2_to_byte_local *) subtask->step->local)->table; struct ucs2_to_byte lookup; struct ucs2_to_byte *entry; unsigned input_value; /* current UCS-2 character */ diff --git a/src/recodext.h b/src/recodext.h index a96647d..aad1340 100644 --- a/src/recodext.h +++ b/src/recodext.h @@ -239,7 +239,7 @@ extern bool recode_interrupted; typedef bool (*Recode_init) (RECODE_STEP, RECODE_CONST_REQUEST, RECODE_CONST_OPTION_LIST, RECODE_CONST_OPTION_LIST); -typedef bool (*Recode_term) (RECODE_STEP, RECODE_CONST_REQUEST); +typedef bool (*Recode_term) (RECODE_STEP); typedef bool (*Recode_transform) (RECODE_SUBTASK); typedef bool (*Recode_fallback) (RECODE_SUBTASK, unsigned); @@ -306,6 +306,7 @@ struct recode_step /* Recoding table. */ void *step_table; + void (*step_table_term_routine)(void *); /* Step specific variables. */ void *local; @@ -536,12 +537,6 @@ struct strip_data const short offset[256 / STRIP_SIZE]; }; -struct ucs2_to_byte - { - recode_ucs2 code; /* UCS-2 value */ - unsigned char byte; /* corresponding byte */ - }; - struct ucs2_to_string { recode_ucs2 code; /* UCS-2 value */ diff --git a/src/request.c b/src/request.c index 922974d..d100f1a 100644 --- a/src/request.c +++ b/src/request.c @@ -238,10 +238,12 @@ add_to_sequence (RECODE_REQUEST request, RECODE_SINGLE single, step->before = single->before; step->after = single->after; step->step_table = single->initial_step_table; + step->step_table_term_routine = NULL; step->step_type = step->step_table ? RECODE_COMBINE_EXPLODE : RECODE_NO_STEP_TABLE; step->transform_routine = single->transform_routine; step->fallback_routine = single->fallback_routine; + step->term_routine = NULL; if (single->init_routine) { @@ -529,6 +531,24 @@ complete_double_ucs2_step (RECODE_OUTER outer, RECODE_STEP step) pair_array, pair_cursor - pair_array, false, reversed); } +static bool +delete_compressed_one_to_many (RECODE_STEP step) +{ + free (step->local); + return true; +} + +static void +delete_step (RECODE_STEP step) +{ + /* Run step destructor, and delete step_table if allocated. */ + if (step->term_routine) + (*step->term_routine) (step); + + if (step->step_table_term_routine != NULL) + (*step->step_table_term_routine) (step->step_table); +} + /*---------------------------------------------------------------------. | Optimize a SEQUENCE of single steps by creating new single steps, if | | this can be done by merging adjacent steps which are simple enough. | @@ -568,6 +588,10 @@ simplify_sequence (RECODE_REQUEST request) && in[1].before == outer->ucs2_charset && in[1].after->data_type == RECODE_STRIP_DATA) { + /* Free old steps before overwriting anything. */ + delete_step (in); + delete_step (in + 1); + /* This is a double UCS-2 step. */ out->before = in[0].before; out->after = in[1].after; @@ -588,6 +612,10 @@ simplify_sequence (RECODE_REQUEST request) && in[0].after == outer->iconv_pivot && in[1].before == outer->iconv_pivot) { + /* Free old steps before overwriting anything. */ + delete_step (in); + delete_step (in + 1); + /* This is a double `iconv' step. */ out->before = in[0].before; out->after = in[1].after; @@ -628,7 +656,7 @@ simplify_sequence (RECODE_REQUEST request) out->before = in->before; out->after = in->after; out->quality = in->quality; - in++; + delete_step (in++); /* Merge in all consecutive one-to-one recodings. */ @@ -643,7 +671,7 @@ simplify_sequence (RECODE_REQUEST request) out->after = in->after; merge_qualities (&out->quality, in->quality); - in++; + delete_step (in++); saved_steps++; } @@ -663,6 +691,12 @@ simplify_sequence (RECODE_REQUEST request) free (accum); out->step_type = RECODE_BYTE_TO_STRING; out->step_table = string; + if (in->step_table_term_routine) + { + out->local = (void *) table; /* Save reference to old table for destructor. */ + out->term_routine = delete_compressed_one_to_many; + } + out->step_table_term_routine = free; out->transform_routine = transform_byte_to_variable; out->after = in->after; merge_qualities (&out->quality, in->quality); @@ -698,6 +732,7 @@ simplify_sequence (RECODE_REQUEST request) { request->sequence_length = 0; saved_steps++; + delete_step (in); } /* Tell the user if something changed. */ @@ -1095,6 +1130,10 @@ recode_new_request (RECODE_OUTER outer) bool recode_delete_request (RECODE_REQUEST request) { + for (RECODE_STEP step = request->sequence_array; + step < request->sequence_array + request->sequence_length; + step++) + delete_step (step); free (request->sequence_array); free (request->work_string); free (request); diff --git a/src/rfc1345.c b/src/rfc1345.c index e2f3c0a..5af5d53 100644 --- a/src/rfc1345.c +++ b/src/rfc1345.c @@ -227,6 +227,13 @@ transform_rfc1345_ucs2 (RECODE_SUBTASK subtask) | Initialise the steps. | `-----------------------*/ +static bool +term_rfc1345 (RECODE_STEP step) +{ + free (step->local); + return true; +} + static bool init_rfc1345 (RECODE_CONST_REQUEST request, RECODE_STEP step, @@ -241,6 +248,7 @@ init_rfc1345 (RECODE_CONST_REQUEST request, local->intro = '&'; step->local = local; + step->term_routine = term_rfc1345; return true; } -- 2.50.1