← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Fix memory leaks with improper wxBaseConfig* usage (model ownership and ownership-transfer with std::unique_ptr)

 

On 6/28/2016 3:30 PM, Michael Steinberg wrote:
>> Potential for errors aside, I think auto makes the code less readable
>> and yes code readability matters.  In this case, I have to go look up
>> the return value of GetNewConfig() because I don't know what an auto
>> type is.  I do know what a wxConfigBase* type is.  I'll just warn
>> everyone now that I am not a big fan of using certain syntax just for
>> the sake of being "C++ correct".  If it makes the code less readable, I
>> will resist committing it.
> I do hear all of you. Let's keep the intent of this patch and the
> implementation aside, I already learned in other side-talks, that having
> an IDE telling you the complete name and more is no default nor a
> desired situation for most developers.
> 
> The right thing to do is using a std::unique_ptr at that spot, that was
> the purpose of the patch and it solves for two leaks that are happening
> in the codebase currently.
> Now I only need to know how I should go about your concerns. Maybe using
> something along the lines of
> 
>     using WX_CONFIG_PTR = std::unique_ptr< wxConfigBase >;
> 
> ?
> I would like to avoid cluttering the source-code with the
> smart-pointer-templates, to avoid people saying "look how long that is,
> a good coder (tm) will just call delete correctly!".
> 
> A sidenote: if I find myself working on some parts of the code, are
> there objections if I silently add "override" specifiers in related
> code? I find these help a lot. Also there are many inline specifiers on
> in-class member-function definitions that could be removed.
> 
> Kind Regards
> Michael

One other thing I forgot to mention is there are coding policy
violations in your patch.  The kicad coding policy can be found in the
developers documentation section of the kicad website [1].  Since it's
your first patch, I'll cut you some slack.  As time goes by the
expectation is that your code follows the coding policy.

Cheers,

Wayne

[1]:http://ci.kicad-pcb.org/job/kicad-doxygen/ws/Documentation/doxygen/html/md_Documentation_development_coding-style-policy.html


References