maria-developers team mailing list archive

[Aleksey Midenkov <midenok@xxxxxxxxx>]

Sergei,

On Sat, Oct 14, 2017 at 9:20 PM, Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
> Hi, Aleksey!
>
> On Oct 13, Aleksey Midenkov wrote:
>> Hello!
>>
>> Do we have to write test cases for such coding errors? Because this is
>> developer factor: either he does such errors or not. What are
>> conditions of getting PR merged?
>
> Generally it's preferrable to have a test case for all bugs where it's
> possible.
>
> In your particular PR, it's unlikely that someone will intentionally add
> memcpy() back. But it's possible that lines around this memcpy would be
> modified in one of the earlier branches, and during the merge git would
> automerge by restoring memcpy(). Or that a person resolving the merge
> conflict would restore the memcpy() without realizing the consequences.
>
> So, yes, test case is better than no test case.
>
> Sometimes a test case is extremely complex or just impossible to create.
> So it's generally at the discretion of the reviewer.
>
> But in this case you're saying that this will be tested in the
> versioning.partition test. Perhaps you can simply push it together with
> versioning.partition test instead of pushing it separately?

Mmm... I'm afraid that it is reproducible only with versioning code.
And I don't know if it will be reproducible when we remove dependence
on native partitioning. I would suggest to move towards C++ generally,
i.e. make constructors instead of memset(), memcpy(). Making it as
coding guide will eventually rectify such cases. Anyway, I guess it's
better with PR than without it... ;)

>
> Regards,
> Sergei
> Chief Architect MariaDB
> and security@xxxxxxxxxxx


--
All the best,
Aleksey Midenkov

Tempesta Technologies