Обсуждение: Patch to eliminate duplicate b64 code from pgcrypto

I am attaching a patch to eliminate duplicate b64_encode and decode
functions from pgcrypto, and to expose those functions for use by
add-ins (I want to use them in Veil).

The patch was made against CVS head today.  It compiles and tests
successfully.  Though I was unable to run pgrypto regression tests, I
did give the b64 encoding a sanity check.

I also added a b64_char() function that is now used from pgcrypto.  This
allowed me to remove the duplicate _base64 character array.

I hope this is the correct place to submit the patch.  The wiki
(http://wiki.postgresql.org/wiki/Submitting_a_Patch) refers to
pgsql-patches but I believe that is now deprecated.


__
Marc

Would someone who understand pgcrypto please review this?
http://archives.postgresql.org/message-id/1216335149.11208.9.camel@bloodnok.com

---------------------------------------------------------------------------

Marc Munro wrote:
-- Start of PGP signed section.
> I am attaching a patch to eliminate duplicate b64_encode and decode
> functions from pgcrypto, and to expose those functions for use by
> add-ins (I want to use them in Veil).
> 
> The patch was made against CVS head today.  It compiles and tests
> successfully.  Though I was unable to run pgrypto regression tests, I
> did give the b64 encoding a sanity check.
> 
> I also added a b64_char() function that is now used from pgcrypto.  This
> allowed me to remove the duplicate _base64 character array.
> 
> I hope this is the correct place to submit the patch.  The wiki
> (http://wiki.postgresql.org/wiki/Submitting_a_Patch) refers to
> pgsql-patches but I believe that is now deprecated.
> 
> 
> __
> Marc

[ Attachment, skipping... ]
-- End of PGP section, PGP failed!

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

On 12/16/08, Bruce Momjian <bruce@momjian.us> wrote:
>  Would someone who understand pgcrypto please review this?

>  Marc Munro wrote:
> > I am attaching a patch to eliminate duplicate b64_encode and decode
>  > functions from pgcrypto, and to expose those functions for use by
>  > add-ins (I want to use them in Veil).

Although your patch achieves the goal, it may make more sense to export
pg_find_encoding() and struct pg_encoding, thus making all encoding
algorithms available externally.

>  > The patch was made against CVS head today.  It compiles and tests
>  > successfully.  Though I was unable to run pgrypto regression tests, I
>  > did give the b64 encoding a sanity check.
>  >
>  > I also added a b64_char() function that is now used from pgcrypto.  This
>  > allowed me to remove the duplicate _base64 character array.

I think this can be avoided by using plain b64_encode() for those 3 bytes.
That step is really not speed critical.

-- 
marko

Oops, forgot to cc my reply to hackers:

On Thu, 2008-12-18 at 01:49 +0200, Marko Kreen wrote:
> On 12/16/08, Bruce Momjian <bruce@momjian.us> wrote:
> >  Would someone who understand pgcrypto please review this?
>
> >  Marc Munro wrote:
> > > I am attaching a patch to eliminate duplicate b64_encode and decode
> >  > functions from pgcrypto, and to expose those functions for use by
> >  > add-ins (I want to use them in Veil).
>
> Although your patch achieves the goal, it may make more sense to export
> pg_find_encoding() and struct pg_encoding, thus making all encoding
> algorithms available externally.

I had a very specific use-case in mind which was to eliminate the need
for Veil to re-implement b64_encode and b64_decode, so I took the path
of least effort.

> >  > The patch was made against CVS head today.  It compiles and tests
> >  > successfully.  Though I was unable to run pgrypto regression tests, I
> >  > did give the b64 encoding a sanity check.
> >  >
> >  > I also added a b64_char() function that is now used from pgcrypto.  This
> >  > allowed me to remove the duplicate _base64 character array.
>
> I think this can be avoided by using plain b64_encode() for those 3 bytes.
> That step is really not speed critical.

You are probably right: I was being somewhat timid and tried to make the
smallest set of changes that were possible.

Unfortunately I don't have the time right now to revisit the patch.  If
you or anyone else would like to re-implement it as you suggest I will
still be a happy camper.   If not, Veil has already re-implemented the
functions, for compatibility with current and older versions of
postgres, so I can live with that too.

Thanks very much for looking at it.  To be honest, I thought that it had
got lost in the transition to the new improved patch process so I was
expecting to have to resubmit it later when I have more time.  Kudos to
Bruce for tracking it down.

__
Marc