From add94b106300b9b28631a4f6f82fb85d68967d0e Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Sun, 27 Nov 2022 16:26:47 +0100 Subject: [PATCH 2/2] review --- src/common/pg_lzcompress.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/common/pg_lzcompress.c b/src/common/pg_lzcompress.c index 248545a108e..bc3388d481f 100644 --- a/src/common/pg_lzcompress.c +++ b/src/common/pg_lzcompress.c @@ -185,11 +185,13 @@ #include "common/pg_lzcompress.h" + /* ---------- * Local definitions * ---------- */ #define PGLZ_MAX_HISTORY_LISTS 8192 /* must be power of 2 */ +/* review: but we only do "==" comparison anyway, so why? also could say 4095 */ #define PGLZ_HISTORY_SIZE 0x0fff /* to avoid compare in iteration */ #define PGLZ_MAX_MATCH 273 @@ -206,6 +208,7 @@ */ typedef struct PGLZ_HistEntry { + /* review: the comment is obsolete, there are indexes, not links) */ int16 next_id; /* links for my hash key's list */ uint16 hist_idx; /* my current hash key */ const unsigned char *pos; /* my input position */ @@ -283,8 +286,7 @@ pglz_hist_idx(const unsigned char *s, uint16 mask) /* ---------- * pglz_hist_add - * - * Adds a new entry to the history table - * and recalculates hash value. + * Adds a new entry to the history table and recalculates hash value. * ---------- */ static inline int16 @@ -350,8 +352,7 @@ pglz_out_tag(unsigned char *dest_ptr, int32 match_len, int32 match_offset) /* ---------- * pglz_compare - * - * Compares two sequences of bytes - * and returns position of first mismatch. + * Compares two sequences of bytes and returns position of first mismatch. * ---------- */ static inline int32 @@ -395,6 +396,9 @@ pglz_find_match(uint16 hist_idx, const unsigned char *input, const unsigned char /* * Traverse the linked history list until a good enough match is found. + * + * review: misplaced comment - this belongs to the while loop further + * down, here we should explain handling of invalid entries and cleanup */ hist_entry_number = &hist_start[hist_idx]; if (*hist_entry_number == INVALID_ENTRY) @@ -581,9 +585,7 @@ pglz_compress(const char *source, int32 slen, char *dest, result_max = (slen / 100) * (100 - need_rate); } else - { result_max = (slen * (100 - need_rate)) / 100; - } /* * Experiments suggest that these hash sizes work pretty well. A large -- 2.38.1