> On 16 Apr 2025, at 13:48, Ashutosh Bapat <ashutosh.bapat....@gmail.com> wrote: > > On Wed, Apr 16, 2025 at 1:57 PM Alexander Kuznetsov > <kuznetso...@altlinux.org> wrote: >> >> Hello everyone, >> >> We've found that EndCompressorZstd() doesn't set cs->private_data to NULL >> after pg_free(), >> unlike other EndCompressor implementations. >> While this doesn't currently cause issues (as the pointer soon gets >> reassigned), >> we recommend fixing this to maintain consistency with other implementations >> and prevent potential future issues. >> >> The patch is attached, would appreciate your thoughts on this change. > > Thanks for the patch. > > The next thing that happens in EndCompressor() is freeing cs itself. > So cs->private_data is not used anywhere, so no harm in the current > code. But it's better to set to NULL since EndCompressorZstd() > wouldn't know how it's being accessed after returning from there. The > other implementation of CompressionState::end() EndCompressorGzip() > calls DeflateCompressorEnd() which also sets cs->private_data > explicitly. So should EndCompressorZstd().
Agreed, while it's perfectly safe today the end method should not make assumptions about the use of the private_data pointer upon return and should leave it set to NULL. -- Daniel Gustafsson
