On 09/10/2014 02:26 PM, Marko Tiikkaja wrote:
> On 9/9/14 11:01 AM, I wrote:
>> On 9/9/14 10:54 AM, Heikki Linnakangas wrote:
>>> So I think this (and the corresponding dearmor code too) should be
>>> refactored to use a StringInfo that gets enlarged as needed, instead of
>>> hoping to guess the size correctly beforehand. To ease review, might
>>> make sense to do that as a separate patch over current sources, and the
>>> main patch on top of that.
>>
>> Yeah, I thought the same thing while writing that code. Thanks, I'll do
>> it that way.
>
> Attached is what I have right now. I started working on the decoding
> part, but it has this piece of code:
>
> /* decode crc */
> if (b64_decode(p + 1, 4, buf) != 3)
> goto out;
>
> which makes the approach a bit uglier. If I did this the same way, I
> would have to create and destroy a StringInfo just for this operation,
> which seems ugly. So I wonder if I shouldn't try and instead keep the
> code closer to what it is in HEAD right now; I could call
> enlargeStringInfo() first, then hand out a pointer to b64_encode (or
> b64_decode()) and finally increment StringInfoData.len by how much was
> actually written. That would keep the code changes a lot smaller, too.
Yeah, that sounds reasonable.
> I'm also not sure why we need to keep a copy of the base64
> encoding/decoding logic instead of exporting it in utils/adt/encode.c.
Good question...
- Heikki