Thread: 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
Attachment
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