Re: Conflicting declarations for b64_encode etc. on Solaris 11.4 Beta

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Postgres bugs <pgsql-bugs(at)postgresql(dot)org>
Cc: Rainer Orth <ro(at)CeBiTec(dot)Uni-Bielefeld(dot)DE>
Subject: Re: Conflicting declarations for b64_encode etc. on Solaris 11.4 Beta
Date: 2018-02-14 06:04:54
Message-ID: 20180214060454.GA4544@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

On Wed, Jan 24, 2018 at 11:16:11AM +0900, Michael Paquier wrote:
> I am not much a fan of using the same function name for both the
> static functions in pgcrypto and encode.c, as well as what is in
> src/common/. In order to avoid any conflicts, why not just changing at
> least the prefix from "b64" to "base64"? That's not completely
> problem-proof for the problem either, as php has a base64_encode for
> example. So my suggestion would be to change the prefix on all
> branches and to append pg_ to all the routines in encode.c for
> consistency. Better naming suggestions welcome.

Attached is an updated patch which does a bit more consistency work. I
have renamed the base64 functions with pg_base64_ as prefix in encode.c,
to avoid any conflicts in encode.c. pgp-armor.c also gets the same
treatment. There is no real point in renaming the other functions is
not necessary, and hex_encode/hex_decode are publicly available, so
renaming them would cause breakage for any callers of them in plugins.

It is a bit sad that both pgcrypto and encode.c hold copies of base64
functions. If one looks closely, they use the same logic for
pg_base64_encode, pg_base64_enc_len and pg_base64_dec_len. Each
pg_b64_decode uses different code paths for error handling, but they
actually have the same conversion logic (see b64_test.c attached which
emulates both, I was too lazy to read and compare b64lookup). One way
to remove the duplication is to extend the _decode API with a path to
not complain with elog and return a status code, then have encode.c use
a wrapper on top of it, but that may not be worth the complication, and
both code copies are not going to change anyway. At the end, just the
attached would do the work.

Rainer, could you double-check if this solves your problem with
Solaris 11? If you expect this OS to be supported, it would be nice if
you could add a buildfarm machine:
https://wiki.postgresql.org/wiki/PostgreSQL_Buildfarm_Howto
https://buildfarm.postgresql.org/
There are currently two machines running Solaris 10, maintained by Dave
Page: protosciurus and castoroides. But there is nothing for Solaris
11.

I am adding that to the next commit fest for consideration as well
under my name. I'll take care of any reviews and/or feedback. I'll add
as well your name as author if I can find your community account, I hope
you don't mind.
--
Michael

Attachment Content-Type Size
solaris-11-compile-v2.patch text/x-diff 3.8 KB
b64_test.c text/x-csrc 1.3 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message PG Bug reporting form 2018-02-14 11:08:08 BUG #15064: Deadlock not detected on standby.
Previous Message Tomas Vondra 2018-02-13 23:53:21 Re: response time is very long in PG9.5.5 using psql or jdbc