Re: Should we add crc32 in libpgport?

On Mon, Jan 16, 2012 at 9:33 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Daniel Farina <daniel(at)heroku(dot)com> writes:
>> Copying CRC32 implementations everywhere is not the worst thing, but I
>> find it inadequately explained why it's necessary for now, at least.
>
> Agreed, but I don't care for your proposed solution (put it in
> libpgport) because that assumes a fact not in evidence, namely that
> external projects have access to libpgport either.

I see. Because of ./configure --disabled-shared is a supported option.

> Is it possible to put enough stuff in pg_crc.h so that external code could
> just include that, perhaps after an extra #define to enable extra code?

Yes. As a nice side effect, we manage to get rid of a self-described
ugly hack, involving exposing the function from libpgport, so outside
the ugly preprocessor dealing, we do score a victory. Related to
that, I have also demoted the symbol from extern to static. There are
a couple of build-process special-cases for utilities like
pg_controldata and pg_resetxlog that are thankfully able to be
removed.

In addition, it seemed pretty weird that this wasn't so much a "port"
(like stub gettimeofday implementations) but rather a function desired
on all platforms -- the degenerate case, where zero platforms have the
function already. So a minor plus of anti-awkwardness of calling it a
'port'.

> As for whether we could drop the existing near-duplicate code in
> contrib/, I think we'd first have to convince ourselves that it was
> functionally identical, because otherwise replacing those versions would
> break existing ltree and hstore indexes.

True. It *is* billed CRC32, so unless there's a bug it *should* be
identical -- but if not, a version bump of the extension/type may be
necessary (do we even know what to do about that, given pg_upgrade?).
I'm not sure what beyond careful inspection (which I haven't done) and
testing a small corpus for binary equivalence what is to be done about
that to be convincing, though. I'll submit the dedup patch
separately, I currently only have ltree done.

See the attached patch. It has a detailed cover letter/comment at the
top of the file.

I have confirmed it applies, builds, and relieves one of my problems
in building xlogdump without access to postgres .o files. I think the
other is surmountable in that project (sprompt.o, which seems hardly
as fundamental). I don't think I've tested the CRC64 path at all, as
it is not used anywhere -- it's sort of there just to occupy symbol
space, as well as I can tell, per its comments ("reserved").

--
fdr