Are pg_control contents really variable-length? - Mailing list pgsql-hackers

From Tom Lane
Subject Are pg_control contents really variable-length?
Date
Msg-id 22771.975114930@sss.pgh.pa.us
Whole thread Raw
List pgsql-hackers
Vadim,

In xlog.c, the declaration of struct ControlFileData says:
   /*    * MORE DATA FOLLOWS AT THE END OF THIS STRUCTURE - locations of data    * dirs    */

Is this comment accurate?  I don't see any sign in the code of placing
extra data after the declared structure.  If you're planning to do it
in future, I think it would be a bad idea.  I'd prefer to see all the
data in that file declared as a fixed-size structure, for two reasons:

1. I'd like to change the statement that reads pg_control into memory
from
   if (read(fd, ControlFile, BLCKSZ) != BLCKSZ)       elog(STOP, "read(\"%s\") failed: %m", ControlFilePath);

to
   if (read(fd, ControlFile, sizeof(ControlFileData)) != sizeof(ControlFileData))       elog(STOP, "read(\"%s\")
failed:%m", ControlFilePath);
 

With the existing code, if one recompiles with a larger BLCKSZ and then
tries to restart without initdb, one gets an unhelpful message about
read() failed --- with no relevant error condition, since early EOF
doesn't set errno --- rather than the helpful complaint about BLCKSZ
mismatch that would come out if we got as far as checking the contents
of the struct.  We've already had one user complaint about this, so it's
far from hypothetical.  If the protocol were to write BLCKSZ amount of
data but only read sizeof(ControlFileData) worth, then we'd have a
better shot at issuing useful rather than useless error messages for
pg_control mismatches.

2. If there is any large amount of data appended to the struct, I'm
a little worried about the data overrunning the BLCKSZ space allocated
for it.  Especially if someone were to reduce BLCKSZ because they were
worried about 8K page writes not being atomic.  I'd prefer to place all
the expected data in the struct and have an explicit test for
sizeof(ControlFileData) <= BLCKSZ. 

Comments?
        regards, tom lane


pgsql-hackers by date:

Previous
From: Lamar Owen
Date:
Subject: Re: OK, that's one LOCALE bug report too many...
Next
From: Tom Lane
Date:
Subject: Re: OK, that's one LOCALE bug report too many...