From 012719d28633a53a45a1c41cde6d226354bed037 Mon Sep 17 00:00:00 2001 From: Yura Sokolov Date: Mon, 24 May 2021 02:16:31 +0300 Subject: [PATCH] fix valgrind alerts valgrind detected some uninitialized memory usage. Looks like shift_lsn one is a real bug. --- src/data.c | 41 ++++++++++++++++++++++++++--------------- src/restore.c | 2 +- src/util.c | 2 +- src/utils/pgut.c | 11 +++++++++++ src/utils/pgut.h | 2 ++ 5 files changed, 41 insertions(+), 17 deletions(-) diff --git a/src/data.c b/src/data.c index 4370bcbbc..d70aae8fd 100644 --- a/src/data.c +++ b/src/data.c @@ -2001,13 +2001,14 @@ send_pages(ConnectionArgs* conn_arg, const char *to_fullpath, const char *from_f { FILE *in = NULL; FILE *out = NULL; - int hdr_num = -1; off_t cur_pos_out = 0; char curr_page[BLCKSZ]; int n_blocks_read = 0; BlockNumber blknum = 0; datapagemap_iterator_t *iter = NULL; int compressed_size = 0; + BackupPageHeader2 *header = NULL; + parray *harray = NULL; /* stdio buffers */ char *in_buf = NULL; @@ -2046,6 +2047,8 @@ send_pages(ConnectionArgs* conn_arg, const char *to_fullpath, const char *from_f setvbuf(in, in_buf, _IOFBF, STDIO_BUFSIZE); } + harray = parray_new(); + while (blknum < file->n_blocks) { PageState page_st; @@ -2063,17 +2066,15 @@ send_pages(ConnectionArgs* conn_arg, const char *to_fullpath, const char *from_f if (!out) out = open_local_file_rw(to_fullpath, &out_buf, STDIO_BUFSIZE); - hdr_num++; - - if (!*headers) - *headers = (BackupPageHeader2 *) pgut_malloc(sizeof(BackupPageHeader2)); - else - *headers = (BackupPageHeader2 *) pgut_realloc(*headers, (hdr_num+1) * sizeof(BackupPageHeader2)); + header = pgut_new0(BackupPageHeader2); + *header = (BackupPageHeader2){ + .block = blknum, + .pos = cur_pos_out, + .lsn = page_st.lsn, + .checksum = page_st.checksum, + }; - (*headers)[hdr_num].block = blknum; - (*headers)[hdr_num].pos = cur_pos_out; - (*headers)[hdr_num].lsn = page_st.lsn; - (*headers)[hdr_num].checksum = page_st.checksum; + parray_append(harray, header); compressed_size = compress_and_backup_page(file, blknum, in, out, &(file->crc), rc, curr_page, calg, clevel, @@ -2098,12 +2099,22 @@ send_pages(ConnectionArgs* conn_arg, const char *to_fullpath, const char *from_f * Add dummy header, so we can later extract the length of last header * as difference between their offsets. */ - if (*headers) + if (parray_num(harray) > 0) { - file->n_headers = hdr_num +1; - *headers = (BackupPageHeader2 *) pgut_realloc(*headers, (hdr_num+2) * sizeof(BackupPageHeader2)); - (*headers)[hdr_num+1].pos = cur_pos_out; + size_t hdr_num = parray_num(harray); + size_t i; + + file->n_headers = (int) hdr_num; /* is it valid? */ + *headers = (BackupPageHeader2 *) pgut_malloc0((hdr_num + 1) * sizeof(BackupPageHeader2)); + for (i = 0; i < hdr_num; i++) + { + header = (BackupPageHeader2 *)parray_get(harray, i); + (*headers)[i] = *header; + pg_free(header); + } + (*headers)[hdr_num] = (BackupPageHeader2){.pos=cur_pos_out}; } + parray_free(harray); /* cleanup */ if (in && fclose(in)) diff --git a/src/restore.c b/src/restore.c index 9594ef0b0..86317596e 100644 --- a/src/restore.c +++ b/src/restore.c @@ -557,8 +557,8 @@ do_restore_or_validate(time_t target_backup_id, pgRecoveryTarget *rt, elog(INFO, "shift LSN: %X/%X", (uint32) (shift_lsn >> 32), (uint32) shift_lsn); - params->shift_lsn = shift_lsn; } + params->shift_lsn = shift_lsn; /* for validation or restore with enabled validation */ if (!params->is_restore || !params->no_validate) diff --git a/src/util.c b/src/util.c index 946957819..87ec36713 100644 --- a/src/util.c +++ b/src/util.c @@ -136,7 +136,7 @@ writeControlFile(ControlFileData *ControlFile, const char *path, fio_location lo #endif /* copy controlFileSize */ - buffer = pg_malloc(ControlFileSize); + buffer = pg_malloc0(ControlFileSize); memcpy(buffer, ControlFile, sizeof(ControlFileData)); /* Write pg_control */ diff --git a/src/utils/pgut.c b/src/utils/pgut.c index a1631b106..eba31faa6 100644 --- a/src/utils/pgut.c +++ b/src/utils/pgut.c @@ -877,6 +877,17 @@ pgut_malloc(size_t size) return ret; } +void * +pgut_malloc0(size_t size) +{ + char *ret; + + ret = pgut_malloc(size); + memset(ret, 0, size); + + return ret; +} + void * pgut_realloc(void *p, size_t size) { diff --git a/src/utils/pgut.h b/src/utils/pgut.h index e6ccbf211..77337a945 100644 --- a/src/utils/pgut.h +++ b/src/utils/pgut.h @@ -59,10 +59,12 @@ extern int pgut_wait(int num, PGconn *connections[], struct timeval *timeout); * memory allocators */ extern void *pgut_malloc(size_t size); +extern void *pgut_malloc0(size_t size); extern void *pgut_realloc(void *p, size_t size); extern char *pgut_strdup(const char *str); #define pgut_new(type) ((type *) pgut_malloc(sizeof(type))) +#define pgut_new0(type) ((type *) pgut_malloc0(sizeof(type))) #define pgut_newarray(type, n) ((type *) pgut_malloc(sizeof(type) * (n))) /*