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

Fri, 24 Nov 2000 20:15:30 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Are pg_control contents really variable-length?
> 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

That was my thought but as you see nothing was done for it, so
feel free to change anything you want there.

Vadim



Sat, 25 Nov 2000 10:56:51 -0800
"Vadim Mikheev" <vmikheev@sectorbase.com>
Re: Are pg_control contents really variable-length?