Hi Rich!
thank you very much for the quick reply.
Yes, Synfig is a quite cool project and it is very pleasant to collaborate
in it.
Your first tip is very useful but it leads to me to some confusion
particularly in the code I'm working on. Let me copy your tip again:
..replace all types used to index arrays with size_t (not int), and
> anything which represents a difference between two pointers with ptrdiff_t.
So, following your first tip I changed the first occurrence of the pointer
difference:
- typedef int difference_type;
+ typedef ptrdiff_t difference_type;
and so, the difference of the two pointers works correctly (better) in this
code:
difference_type operator-(const self_type &rhs)const
{ assert(data_); return ((char*)data_-(char*)rhs.data_-1)/pitch_+1; }
Neat!
But, immediately I go to the following code:
self_type operator+(const difference_type &rhs)const
{
assert(data_);
self_type ret(*this);
ret.inc(rhs);
return ret;
}
Where difference_type is used.
So I think: if difference_type is now ptrdiff_t, then the ret.inc(rhs);
call should expect to receive the same type, right?
so I looked the code of the inc function and replaced the int with the same
type it is used to pass it:
-void inc(int n) { assert(data_); data_ = (pointer)((char*)data_ +
n*pitch_); }
+void inc(difference_type n) { assert(data_); data_ =
(pointer)((char*)data_ + n*pitch_); }
but wait a second: inc is doing a pointer increment of n*pitch bytes,
shouldn't it be a size_t type???
Moreover, the operator[](int i) isn't doing something very similar?
shouldn't I use size_t there?
-reference operator[](int i)const { assert(data_); return *(pointer)(
(char*)data_+pitch_*i ); }
+ reference operator[](size_t i)const { assert(data_); return *(pointer)(
(char*)data_+pitch_*i ); }
This confusion with ptrdiff_t and size_t is getting me crazy. In this
particular case:
if I use void inc(size_t) instead of void inc(ptrdiff_t) would it convert
the values correctly?
I've done changes in the direction of replace with ptrdiff_t ad size_t
where I believe it should be
in the generic_pen and everything leads to a instant crash. I'll post a
diff later if you want but I first want to
understand when use size_t or ptrdiff_t
The +1,-1 thing is very strange to me too. It seems to be some sort of hack
to fix other rounding error
that it might happen in the procedures that uses the generic pen. It has
been there since 2005 and nobody has changed it.
Removing it doesn't seem to hurt at the render but the checkerboard layer
seems to be displaced one pixel down-right more than it is
currently displaced (it has a bug of location of the checkerboard)
Last word. The reason of looking the code is that when we are rendering one
special kind of layer (Warp) it needs to render the layers below
in a huge size and then, since a certain image size, the operator[] form
the generic pen fails with BAD_ACCESS. The key to make me think on a code
bug was that it
always crashed when the size of the image in bytes is near the size of the
maximum int value and so the int is overflow to negative and then it is
trying to access one illegal address.
Well, this is my theory. ^__^
Thank you very much for reading!
Cheers!
2012/9/13 Rich Talbot-Watkins <[email protected]>
> Hi Carlos!
>
> Looks like a really cool project! I haven't had so much time to study the
> code (at work at the moment), but here are a few tips in general.
>
> The main thing to do for 64-bit compliant code is to replace all types
> used to index arrays with size_t (not int), and anything which represents a
> difference between two pointers with ptrdiff_t. Since you're already
> typedef'ing a "difference type", it won't be too much work to identify and
> change these references.
>
> By the way, I don't quite understand the intention of these two bits of
> code:
>
> difference_type operator-(const self_type &rhs)const
> { assert(data_); return ((char*)data_-(char*)rhs.data_**-1)/pitch_+1;
> }
>
> and:
>
> difference_type operator-(const self_type &rhs)const
> {
> assert(data_);
> assert(pitch_==rhs.pitch_);
> int ptr_diff=(char*)data_-(char*)**rhs.data_-1;
> return difference_type(ptr_diff%**pitch_/sizeof(value_type)+1,**
> ptr_diff/pitch_);
> }
>
> specifically: why the -1 / +1 in the calculation? Is this correct?
>
> The last thing that remains would be to trap the overflow when attempting
> to 'new' the array in the first place. On a 32-bit system where size_t is
> an unsigned int32, the following would probably silently fail:
>
> int w = 0x10000;
> int h = 0x10000;
> size_t size = w * h * sizeof(float);
>
> because c++ allows unsigned types to overflow without error (the result
> just wraps around to 0). So you should check the result afterwards, so
> that you can fail gracefully on 32-bit systems (which would not be able to
> contain the array in any case):
>
> if (size / (h * sizeof(float)) != w)
> {
> // result overflowed
> }
>
> (Google 'c++ unsigned overflow' for more information on this than is
> probably healthy!)
>
> Good luck, and if you have any more specific questions, get in touch!
>
> All the best,
> Rich
>
>
>
> On 13/09/2012 11:53, Carlos López González wrote:
>
>> Hi Rich!
>> First of all thanks for offer your knowledge to help me in this matter.
>> The question I threw to David was the simplest question about pointers
>> that I was able to extract from the problem I'm facing. The question isn't
>> that simple like that.
>>
>> First, we are dealing with a template class that is used to store an
>> image. So we have a pointer to a generic class T and two values of the size
>> of the image w and y. The code stores using the 'new' statement an
>> allocation of memory of w*h*sizeof(T). For some working cases, the value of
>> that product is higher than the maximum value of an int type. In the code
>> it is needed to access the data in steps of bytes (that later are type
>> casted to T*) so I need to do the pointer arithmetics in bytes that cannot
>> be done using 'int' types obviously for the reason above. The T* pointer is
>> casted first to char* before doing the arithmetics and then casted back
>> again to T* afterwards.
>>
>> So my initial question is: what type should I use on the variables that
>> are used on the char* arithmetics so they don't overflow like 'int' does?
>>
>> The code of the template is here:
>>
>> https://github.com/synfig/**synfig/blob/master/ETL/ETL/_**pen.h<https://github.com/synfig/synfig/blob/master/ETL/ETL/_pen.h>
>>
>> I'm sorry for not having a simpler example.
>>
>> Thank you very much for taking time on it!
>> Greetings
>> PS: I CC this email to synfig-devl mailing list
>>
>
>
--
Carlos
http://synfig.org
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Synfig-devl mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/synfig-devl
