Tarantool development patches archive
 help / color / mirror / Atom feed
From: Sergey Nikiforov via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Alexander Turenko <alexander.turenko@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org,
	Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH v7] base64: fix decoder output buffer overrun (reads)
Date: Tue, 9 Mar 2021 13:09:05 +0300	[thread overview]
Message-ID: <b58f6948-c362-a908-7fee-5db2164e240a@tarantool.org> (raw)
In-Reply-To: <20210302230820.4kiad5inxhbitkkl@tkn_work_nb>

On 03.03.2021 2:08, Alexander Turenko wrote:
>> -static int
>> -base64_decode_block(const char *in_base64, int in_len,
>> -		    char *out_bin, int out_len,
>> -		    struct base64_decodestate *state)
>> +int
>> +base64_decode(const char *in_base64, int in_len,
>> +	      char *out_bin, int out_len)
>>   {
>>   	const char *in_pos = in_base64;
>>   	const char *in_end = in_base64 + in_len;
>>   	char *out_pos = out_bin;
>>   	char *out_end = out_bin + out_len;
>>   	int fragment;
>> +	char curr_byte;
> 
> AFAIR, nothing prevent us from using `*out_pos`. I don't see a reason to
> introduce `curr_byte`. It was necessary in v6 to store the initial
> state for a block.

I have removed this local var in v8.

Initially I planned to send patch series with this local var added in 
second patch and performance-optimized (w/o unnecessarily checks) 
version of decode function in third but my benchmarks have shown 
controversial effects. Adding this local var "by itself" actually 
decreases performance on one of my test machines but improves on another 
one. Optimized decode function, however, benefits from this local var 
but not when output buffer is large enough. And effect is different on 
another machine...

I haved saved optimized code (nothing really new here) and benchmark 
locally to work on them further later on.

      reply	other threads:[~2021-03-09 10:09 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-01 12:31 Sergey Nikiforov via Tarantool-patches
2021-03-02 23:08 ` Alexander Turenko via Tarantool-patches
2021-03-09 10:09   ` Sergey Nikiforov via Tarantool-patches [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b58f6948-c362-a908-7fee-5db2164e240a@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=alexander.turenko@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --cc=void@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v7] base64: fix decoder output buffer overrun (reads)' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox