mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-08-23 13:10:41 +02:00
Code review for pg_verify_checksums.c.
Use postgres_fe.h, since this is frontend code. Pretend that we've heard of project style guidelines for, eg, #include order. Use BlockNumber not int arithmetic for block numbers, to avoid misbehavior with relations exceeding 2^31 blocks. Avoid an unnecessary strict-aliasing warning (per report from Michael Banck). Const-ify assorted stuff. Avoid scribbling on the output of readdir() -- perhaps that's safe in practice, but POSIX forbids it, and this code has so far earned exactly zero credibility portability-wise. Editorialize on an ambiguously-worded message. I did not touch the problem of the "buf" local variable being possibly insufficiently aligned; that's not specific to this code, and seems like it should be fixed as part of a different, larger patch. Discussion: https://postgr.es/m/1535618100.1286.3.camel@credativ.de
This commit is contained in:
parent
36343e59b5
commit
d787af7bad
@ -7,23 +7,20 @@
|
|||||||
*
|
*
|
||||||
* src/bin/pg_verify_checksums/pg_verify_checksums.c
|
* src/bin/pg_verify_checksums/pg_verify_checksums.c
|
||||||
*/
|
*/
|
||||||
|
#include "postgres_fe.h"
|
||||||
|
|
||||||
#define FRONTEND 1
|
#include <dirent.h>
|
||||||
|
#include <sys/stat.h>
|
||||||
|
#include <unistd.h>
|
||||||
|
|
||||||
#include "postgres.h"
|
|
||||||
#include "catalog/pg_control.h"
|
#include "catalog/pg_control.h"
|
||||||
#include "common/controldata_utils.h"
|
#include "common/controldata_utils.h"
|
||||||
#include "getopt_long.h"
|
#include "getopt_long.h"
|
||||||
|
#include "pg_getopt.h"
|
||||||
#include "storage/bufpage.h"
|
#include "storage/bufpage.h"
|
||||||
#include "storage/checksum.h"
|
#include "storage/checksum.h"
|
||||||
#include "storage/checksum_impl.h"
|
#include "storage/checksum_impl.h"
|
||||||
|
|
||||||
#include <sys/stat.h>
|
|
||||||
#include <dirent.h>
|
|
||||||
#include <unistd.h>
|
|
||||||
|
|
||||||
#include "pg_getopt.h"
|
|
||||||
|
|
||||||
|
|
||||||
static int64 files = 0;
|
static int64 files = 0;
|
||||||
static int64 blocks = 0;
|
static int64 blocks = 0;
|
||||||
@ -36,7 +33,7 @@ static bool verbose = false;
|
|||||||
static const char *progname;
|
static const char *progname;
|
||||||
|
|
||||||
static void
|
static void
|
||||||
usage()
|
usage(void)
|
||||||
{
|
{
|
||||||
printf(_("%s verifies data checksums in a PostgreSQL database cluster.\n\n"), progname);
|
printf(_("%s verifies data checksums in a PostgreSQL database cluster.\n\n"), progname);
|
||||||
printf(_("Usage:\n"));
|
printf(_("Usage:\n"));
|
||||||
@ -52,7 +49,7 @@ usage()
|
|||||||
printf(_("Report bugs to <pgsql-bugs@postgresql.org>.\n"));
|
printf(_("Report bugs to <pgsql-bugs@postgresql.org>.\n"));
|
||||||
}
|
}
|
||||||
|
|
||||||
static const char *skip[] = {
|
static const char *const skip[] = {
|
||||||
"pg_control",
|
"pg_control",
|
||||||
"pg_filenode.map",
|
"pg_filenode.map",
|
||||||
"pg_internal.init",
|
"pg_internal.init",
|
||||||
@ -61,9 +58,9 @@ static const char *skip[] = {
|
|||||||
};
|
};
|
||||||
|
|
||||||
static bool
|
static bool
|
||||||
skipfile(char *fn)
|
skipfile(const char *fn)
|
||||||
{
|
{
|
||||||
const char **f;
|
const char *const *f;
|
||||||
|
|
||||||
if (strcmp(fn, ".") == 0 ||
|
if (strcmp(fn, ".") == 0 ||
|
||||||
strcmp(fn, "..") == 0)
|
strcmp(fn, "..") == 0)
|
||||||
@ -76,12 +73,12 @@ skipfile(char *fn)
|
|||||||
}
|
}
|
||||||
|
|
||||||
static void
|
static void
|
||||||
scan_file(char *fn, int segmentno)
|
scan_file(const char *fn, BlockNumber segmentno)
|
||||||
{
|
{
|
||||||
char buf[BLCKSZ];
|
char buf[BLCKSZ];
|
||||||
PageHeader header = (PageHeader) buf;
|
PageHeader header = (PageHeader) buf;
|
||||||
int f;
|
int f;
|
||||||
int blockno;
|
BlockNumber blockno;
|
||||||
|
|
||||||
f = open(fn, O_RDONLY | PG_BINARY);
|
f = open(fn, O_RDONLY | PG_BINARY);
|
||||||
if (f < 0)
|
if (f < 0)
|
||||||
@ -102,21 +99,21 @@ scan_file(char *fn, int segmentno)
|
|||||||
break;
|
break;
|
||||||
if (r != BLCKSZ)
|
if (r != BLCKSZ)
|
||||||
{
|
{
|
||||||
fprintf(stderr, _("%s: short read of block %d in file \"%s\", got only %d bytes\n"),
|
fprintf(stderr, _("%s: short read of block %u in file \"%s\", got only %d bytes\n"),
|
||||||
progname, blockno, fn, r);
|
progname, blockno, fn, r);
|
||||||
exit(1);
|
exit(1);
|
||||||
}
|
}
|
||||||
blocks++;
|
blocks++;
|
||||||
|
|
||||||
/* New pages have no checksum yet */
|
/* New pages have no checksum yet */
|
||||||
if (PageIsNew(buf))
|
if (PageIsNew(header))
|
||||||
continue;
|
continue;
|
||||||
|
|
||||||
csum = pg_checksum_page(buf, blockno + segmentno * RELSEG_SIZE);
|
csum = pg_checksum_page(buf, blockno + segmentno * RELSEG_SIZE);
|
||||||
if (csum != header->pd_checksum)
|
if (csum != header->pd_checksum)
|
||||||
{
|
{
|
||||||
if (ControlFile->data_checksum_version == PG_DATA_CHECKSUM_VERSION)
|
if (ControlFile->data_checksum_version == PG_DATA_CHECKSUM_VERSION)
|
||||||
fprintf(stderr, _("%s: checksum verification failed in file \"%s\", block %d: calculated checksum %X but expected %X\n"),
|
fprintf(stderr, _("%s: checksum verification failed in file \"%s\", block %u: calculated checksum %X but block contains %X\n"),
|
||||||
progname, fn, blockno, csum, header->pd_checksum);
|
progname, fn, blockno, csum, header->pd_checksum);
|
||||||
badblocks++;
|
badblocks++;
|
||||||
}
|
}
|
||||||
@ -130,7 +127,7 @@ scan_file(char *fn, int segmentno)
|
|||||||
}
|
}
|
||||||
|
|
||||||
static void
|
static void
|
||||||
scan_directory(char *basedir, char *subdir)
|
scan_directory(const char *basedir, const char *subdir)
|
||||||
{
|
{
|
||||||
char path[MAXPGPATH];
|
char path[MAXPGPATH];
|
||||||
DIR *dir;
|
DIR *dir;
|
||||||
@ -146,7 +143,7 @@ scan_directory(char *basedir, char *subdir)
|
|||||||
}
|
}
|
||||||
while ((de = readdir(dir)) != NULL)
|
while ((de = readdir(dir)) != NULL)
|
||||||
{
|
{
|
||||||
char fn[MAXPGPATH + 1];
|
char fn[MAXPGPATH];
|
||||||
struct stat st;
|
struct stat st;
|
||||||
|
|
||||||
if (skipfile(de->d_name))
|
if (skipfile(de->d_name))
|
||||||
@ -161,9 +158,10 @@ scan_directory(char *basedir, char *subdir)
|
|||||||
}
|
}
|
||||||
if (S_ISREG(st.st_mode))
|
if (S_ISREG(st.st_mode))
|
||||||
{
|
{
|
||||||
|
char fnonly[MAXPGPATH];
|
||||||
char *forkpath,
|
char *forkpath,
|
||||||
*segmentpath;
|
*segmentpath;
|
||||||
int segmentno = 0;
|
BlockNumber segmentno = 0;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Cut off at the segment boundary (".") to get the segment number
|
* Cut off at the segment boundary (".") to get the segment number
|
||||||
@ -171,7 +169,8 @@ scan_directory(char *basedir, char *subdir)
|
|||||||
* fork boundary, to get the relfilenode the file belongs to for
|
* fork boundary, to get the relfilenode the file belongs to for
|
||||||
* filtering.
|
* filtering.
|
||||||
*/
|
*/
|
||||||
segmentpath = strchr(de->d_name, '.');
|
strlcpy(fnonly, de->d_name, sizeof(fnonly));
|
||||||
|
segmentpath = strchr(fnonly, '.');
|
||||||
if (segmentpath != NULL)
|
if (segmentpath != NULL)
|
||||||
{
|
{
|
||||||
*segmentpath++ = '\0';
|
*segmentpath++ = '\0';
|
||||||
@ -184,11 +183,11 @@ scan_directory(char *basedir, char *subdir)
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
forkpath = strchr(de->d_name, '_');
|
forkpath = strchr(fnonly, '_');
|
||||||
if (forkpath != NULL)
|
if (forkpath != NULL)
|
||||||
*forkpath++ = '\0';
|
*forkpath++ = '\0';
|
||||||
|
|
||||||
if (only_relfilenode && strcmp(only_relfilenode, de->d_name) != 0)
|
if (only_relfilenode && strcmp(only_relfilenode, fnonly) != 0)
|
||||||
/* Relfilenode not to be included */
|
/* Relfilenode not to be included */
|
||||||
continue;
|
continue;
|
||||||
|
|
||||||
@ -247,7 +246,7 @@ main(int argc, char *argv[])
|
|||||||
DataDir = optarg;
|
DataDir = optarg;
|
||||||
break;
|
break;
|
||||||
case 'r':
|
case 'r':
|
||||||
if (atoi(optarg) <= 0)
|
if (atoi(optarg) == 0)
|
||||||
{
|
{
|
||||||
fprintf(stderr, _("%s: invalid relfilenode specification, must be numeric: %s\n"), progname, optarg);
|
fprintf(stderr, _("%s: invalid relfilenode specification, must be numeric: %s\n"), progname, optarg);
|
||||||
exit(1);
|
exit(1);
|
||||||
|
Loading…
Reference in New Issue
Block a user