Now I see that BufFileCreateShared() has similar problem with file->name.
More generic problem I see is that the common initialization of BufFile is
repeated a few times. The attached patch tries to improve that (it also fixes
the duplicate allocation of file->name).
Tatsuo Ishii <[email protected]> wrote:
> > Memory is allocated twice for "file" and "files" variables. Possible fix:
> >
> > diff --git a/src/backend/storage/file/buffile.c
> > b/src/backend/storage/file/buffile.c
> > index d8a18dd3dc..00f61748b3 100644
> > --- a/src/backend/storage/file/buffile.c
> > +++ b/src/backend/storage/file/buffile.c
> > @@ -277,10 +277,10 @@ BufFileCreateShared(SharedFileSet *fileset, const
> > char *name)
> > BufFile *
> > BufFileOpenShared(SharedFileSet *fileset, const char *name)
> > {
> > - BufFile *file = (BufFile *) palloc(sizeof(BufFile));
> > + BufFile *file;
> > char segment_name[MAXPGPATH];
> > Size capacity = 16;
> > - File *files = palloc(sizeof(File) * capacity);
> > + File *files;
> > int nfiles = 0;
> >
> > file = (BufFile *) palloc(sizeof(BufFile));
>
> Good catch. Thanks.
>
> Best regards,
> --
> Tatsuo Ishii
> SRA OSS, Inc. Japan
> English: http://www.sraoss.co.jp/index_en.php
> Japanese:http://www.sraoss.co.jp
>
--
Antonin Houska
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26, A-2700 Wiener Neustadt
Web: https://www.cybertec-postgresql.com
diff --git a/src/backend/storage/file/buffile.c b/src/backend/storage/file/buffile.c
index d8a18dd3dc..e345d008b5 100644
--- a/src/backend/storage/file/buffile.c
+++ b/src/backend/storage/file/buffile.c
@@ -99,6 +99,7 @@ struct BufFile
char buffer[BLCKSZ];
};
+static BufFile *makeBufFileCommon(int nfiles);
static BufFile *makeBufFile(File firstfile);
static void extendBufFile(BufFile *file);
static void BufFileLoadBuffer(BufFile *file);
@@ -106,21 +107,16 @@ static void BufFileDumpBuffer(BufFile *file);
static int BufFileFlush(BufFile *file);
static File MakeNewSharedSegment(BufFile *file, int segment);
-
/*
- * Create a BufFile given the first underlying physical file.
- * NOTE: caller must set isInterXact if appropriate.
+ * Create BufFile and perform the common initialization.
*/
static BufFile *
-makeBufFile(File firstfile)
+makeBufFileCommon(int nfiles)
{
BufFile *file = (BufFile *) palloc(sizeof(BufFile));
- file->numFiles = 1;
- file->files = (File *) palloc(sizeof(File));
- file->files[0] = firstfile;
- file->offsets = (off_t *) palloc(sizeof(off_t));
- file->offsets[0] = 0L;
+ file->numFiles = nfiles;
+ file->offsets = (off_t *) palloc0(sizeof(off_t) * nfiles);
file->isInterXact = false;
file->dirty = false;
file->resowner = CurrentResourceOwner;
@@ -128,6 +124,21 @@ makeBufFile(File firstfile)
file->curOffset = 0L;
file->pos = 0;
file->nbytes = 0;
+
+ return file;
+}
+
+/*
+ * Create a BufFile given the first underlying physical file.
+ * NOTE: caller must set isInterXact if appropriate.
+ */
+static BufFile *
+makeBufFile(File firstfile)
+{
+ BufFile *file = makeBufFileCommon(1);
+
+ file->files = (File *) palloc(sizeof(File));
+ file->files[0] = firstfile;
file->readOnly = false;
file->fileset = NULL;
file->name = NULL;
@@ -246,23 +257,12 @@ BufFileCreateShared(SharedFileSet *fileset, const char *name)
{
BufFile *file;
- file = (BufFile *) palloc(sizeof(BufFile));
+ file = makeBufFileCommon(1);
file->fileset = fileset;
file->name = pstrdup(name);
- file->numFiles = 1;
file->files = (File *) palloc(sizeof(File));
file->files[0] = MakeNewSharedSegment(file, 0);
- file->offsets = (off_t *) palloc(sizeof(off_t));
- file->offsets[0] = 0L;
- file->isInterXact = false;
- file->dirty = false;
- file->resowner = CurrentResourceOwner;
- file->curFile = 0;
- file->curOffset = 0L;
- file->pos = 0;
- file->nbytes = 0;
file->readOnly = false;
- file->name = pstrdup(name);
return file;
}
@@ -286,4 +286,3 @@
- file = (BufFile *) palloc(sizeof(BufFile));
files = palloc(sizeof(File) * capacity);
/*
@@ -317,16 +316,8 @@ BufFileOpenShared(SharedFileSet *fileset, const char *name)
(errcode_for_file_access(),
errmsg("could not open BufFile \"%s\"", name)));
- file->numFiles = nfiles;
+ file = makeBufFileCommon(nfiles);
file->files = files;
- file->offsets = (off_t *) palloc0(sizeof(off_t) * nfiles);
- file->isInterXact = false;
- file->dirty = false;
- file->resowner = CurrentResourceOwner; /* Unused, can't extend */
- file->curFile = 0;
- file->curOffset = 0L;
- file->pos = 0;
- file->nbytes = 0;
file->readOnly = true; /* Can't write to files opened this way */
file->fileset = fileset;
file->name = pstrdup(name);
