On Fri, Nov 20, 2020 at 11:17:44PM +0100, Daniel Gustafsson wrote:
> On 20 Nov 2020, at 01:33, Michael Paquier <michael@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