On 13.12.25 01:41, Sami Imseih wrote:
Thanks for the updates!

- Less fwrite() and fread(), more read_chunk() and write_chunk().  We
are exposing these APIs, let's use them.

oops. That totally slipped my mind :( sorry about that.

- The callbacks are renamed, to be more generic: "finish" for the
end-of-operation actions and to/from_serialized_data.

At first I wasn’t a fan of the name “finish” for the callback.
I was thinking of calling it “finish_auxiliary”. But, we’re not
forcing callbacks to be used together, and there could perhaps
be cases where “finish" can be used on its own, so this is fine by me.

I made some changes as well, in v8:

1/ looks like b4cbc106a6ce snuck into v7. I fixed that.

2/ After looking this over, I realized that “extra” and “auxiliary”
were being used interchangeably. To avoid confusion, I replaced all
instances of “extra” with “auxiliary" in both the comments and
macros, i.e. TEST_CUSTOM_AUX_DATA_DESC

The function test_custom_stats_var_from_serialized_data() takes an argument of type

    const PgStatShared_Common *header

which is then later cast

    entry = (PgStatShared_CustomVarEntry *) header;

where entry is defined as

    PgStatShared_CustomVarEntry *entry;

So you are losing the const qualification here.

But fixing that by adding the const qualification to entry would not work because what entry points to is later modified:

    entry->description = InvalidDsaPointer;

So the header argument of the function should not be const qualified.

But the signature of that function is apparently determined by this new callbacks API, so it cannot be changed in isolation.

So it seems to me that either the callbacks API needs some adjustments, or this particular implementation of the callback function is incorrect.



Reply via email to