kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxxxx>]

On 12/5/2011 2:21 PM, Dick Hollenbeck wrote:
> On 12/05/2011 12:13 PM, Wayne Stambaugh wrote:
>> On 12/5/2011 11:55 AM, Dick Hollenbeck wrote:
>>> On 12/05/2011 10:35 AM, Wayne Stambaugh wrote:
>>>> On 12/5/2011 9:48 AM, Dick Hollenbeck wrote:
>>>>> On 12/05/2011 08:02 AM, Wayne Stambaugh wrote:
>>>>>> On 12/5/2011 12:30 AM, Dick Hollenbeck wrote:
>>>>>>> Wayne,
>>>>>>>
>>>>>>> I think the lines involving operator new() may blow up at runtime when the corresponding
>>>>>>> delete [] is called.
>>>>>> Dick,
>>>>>>
>>>>>> I'm not using
>>>>>>
>>>>>> delete[] foo;        // This is not the same as delete foo[nn];
>>>>>>
>>>>>> where the contents of foo are allocated using
>>>>>>
>>>>>> foo = new();
>>>>>>
>>>>>> I'm using
>>>>>>
>>>>>> delete foo[nn];      // This is the same as tmp* = foo[nn]; delete tmp;
>>>>>>
>>>>>> where the contents of the array of foo pointers is assigned using
>>>>>>
>>>>>> foo[nn] = new( sizeof(foo) );
>>>>>>
>>>>>> In the case of m_BoardSide it is defined in the board class as
>>>>>>
>>>>>> MATRIX_CELL* m_BoardSide[MAX_SIDES_COUNT];
>>>>>>
>>>>>> so I'm just filling the array of pointers with the new() operator so delete()
>>>>>> should work properly.
>>>>>>
>>>>>>> Have you tested this code?
>>>>>> I ran the auto router and didn't it segfault and it generated the same results
>>>>>> as the old code so it appears to be correct.
>>>>>>
>>>>>> Wayne
>>>>> Ok.  A related concern on this line:
>>>>>
>>>>>         /* allocate Board & initialize everything to empty */
>>>>>         m_BoardSide[kk] = (MATRIX_CELL*) operator new( ii * sizeof(MATRIX_CELL) );
>>>>>
>>>>>
>>>>> My understanding is that the old code also did not run the constructor MATRIX_CELL on each
>>>>> element within the range [0] < [ii-1].
>>>> Yep.  I originally tried to use new[] but calling the constructor broke the
>>>> auto router.  The original ZMalloc code did not call the constructor either so
>>>> using new( count ) was the best short term solution.  I know it is not the most
>>>> elegant solution but my main goal was to stop using malloc() because no one was
>>>> testing if the result of the allocation was NULL.  At least new() will raise an
>>>> assertion that will get caught further up the stack by wxWidgets.  It may still
>>>> crash but a least you will know the the type of assertion that cause the
>>>> problem even in release builds.
>>> Your concept is sound.  I just wanted to point out that the behavior of the new code is
>>> different, and that the comment seems out of date.  No initialization is taking place,
>>> whereas before, there was.  The memory was being zeroed out.  This makes the false comment
>>> even more dangerous, because it is misleading one into thinking something equivalent is
>>> happening.
>> Is this the comment?
>>
>> /* allocate Board & initialize everything to empty */
>>
>> I couldn't find anything else in the Doxygen comments about the memory being
>> zeroed.  If this is the comment, I'll take out "initialize everything to empty"
>> in my next commit.
>>
>> Wayne
> 
> 
> /*
>  * My memory allocation, memory space is cleared
>  */
> void* MyZMalloc( size_t nb_octets )
> {
>     void* pt_mem = MyMalloc( nb_octets );
> 
>     if( pt_mem )
>         memset( pt_mem, 0, nb_octets );
>     return pt_mem;
> }
> 
> 
> the above, from old common.cpp version 2224.
> 
> 
> So I'm just saying that if it is your intent to initialize the memory to zeroes, then do
> it.  If you are certain that the memory is fine NOT CLEARED TO ZEROES, then change the
> comment, so somebody knows that something is different.  Because it is different.

I'll go ahead a zero the allocated memory.  This way it is the same as
the previous behavior.

Wayne

> 
> 
> Also there is a chance that you randomly got memory initialized to zeroes from your
> operator new(), THIS TIME, in your testing, and got lucky.  But Joe KiCad user does not
> get memory initialized to zeroes, and wishes he did.
> 
> 
> Thanks
> 
> Dick
> 
> 
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>