On Fri, Nov 20, 2020 at 11:17:44PM +0100, Daniel Gustafsson wrote: > On 20 Nov 2020, at 01:33, Michael Paquier <mich...@paquier.xyz> wrote: >> knowing that we still need to deal with the OOM failure handling >> and pass down the error to the callers playing with SHA2, it feels >> like the most consistent API to me for the frontend and the backend. > > For the backend I'd prefer an API where the allocation worked like palloc, if > not then it's a straight exit through the giftshop. But if we want an API for > both the frontend and backend, I guess this is what we'll have to do.
The fallback implementations can somewhat achieve that as we know all the internals of the structures used. > Ok, then at least I think we should use PG_CRYPTOHASH_H to be more consistent > with the tree, and since leading underscores in C are problematic spec-wise. Makes sense. Will fix. >> That was the least intrusive option I could figure out. Two other >> options I have thought about: >> - Compile those fallback implementations independently and publish the >> internals in a separate header, but nobody is going to need that if we >> have a generic entry point. >> - Include the internals of each implementation in cryptohash.c, but >> this bloats the file with more implementations added (HMAC and MD5 >> still need to be added on top of SHA2), and it messes up with the >> existing copyright entries. >> So splitting and just including them sounds like the cleanest option >> of the set. > > Personally I think the first option of using an internal header seems cleaner, > but MMV so I'll leave it to others to weigh in too. What you meant and what I meant was slightly different here. I meant publishing a header in src/include/common/ that would get installed, and I'd rather avoid that. And you mean to have the header for local consumption in src/common/. I would be fine with your third option as well. Your suggestion is more consistent with what we do for the rest of src/common/ and libpq actually. So I don't mind switching to that. -- Michael
signature.asc
Description: PGP signature
