On 10.05.23 20:10, Andres Freund wrote:
>> So if you want to understand what smgrzeroextend() does, you need to
>> mentally combine the documentation of three different functions. Could we
>> write documentation for each function that stands on its own? And document
>> the function arguments, like what does blocknum and nblocks mean?
> I guess it couldn't hurt. But if we go down that route, we basically need to
> rewrite all the function headers in smgr.c, I think.
I took a stab at this, going through the function comments in smgr.c and
md.c and try to make some things easier to follow.
- Took at out the weird leading tabs in the older comments.
- Rephrased some comments so that smgr.c is more like an API
documentation and md.c documents what that particular instance of that
API does.
- Move the *extend and *zeroextend functions to a more sensible place
among all the functions. Especially since *write and *extend are very
similar, it makes sense to have them close together. This way, it's
also easier to follow "this function is like that function" comments.
- Also moved mdwriteback(), which was in some completely odd place.
- Added comments for function arguments that were previously not documented.
- Reworded the comments for *extend and *zeroextend to make more sense
relative to each other.
- I left this for smgrzeroextend():
FIXME: why both blocknum and nblocks
Like, what happens when blocknum is not the block right after the last
existing block? Do you get an implicit POSIX hole between the end of
the file and the specified block and then an explicit hole for the next
nblocks? We should be clear here what the designer of this API intended.
The name smgrzeroextend is not great. The "zero" isn't what
distinguishes it from smgrextend.