On Wed, Nov 24, 2021 at 10:55 AM Michael Paquier <michael@paquier.xyz> wrote:
On Mon, Nov 22, 2021 at 09:02:47AM -0500, Robert Haas wrote: > On Mon, Nov 22, 2021 at 12:46 AM Jeevan Ladhe > <jeevan.ladhe@enterprisedb.com> wrote: >> Fair enough. But, still I have a doubt in mind what benefit would that >> really bring to us here, because we are immediately also freeing the >> lz4buf without using it anywhere. > > Yeah, I'm also doubtful about that. If we're freeng the compression > context, we shouldn't need to guarantee that it's in any particular > state before doing so. Why would any critical cleanup be part of > LZ4F_compressEnd() rather than LZ4F_freeCompressionContext()? The > point of LZ4F_compressEnd() is to make sure all of the output bytes > get written, and it would be stupid to force people to write the > output bytes even when they've decided that they no longer care about > them due to some error.
Hmm. I have double-checked all that, and I agree that we could just skip LZ4F_compressEnd() in this error code path. From what I can see in the upstream code, what we have now is not broken either, but the compressEnd() call does some work that's not needed here.
Yes I agree that we are not broken, but as you said we are doing some