> On Feb 4, 2020, at 20:43, Fred Kiefer <fredkie...@gmx.de> wrote:
>
>
>
>> Am 04.02.2020 um 11:21 schrieb Sergii Stoian <stoyan...@gmail.com>:
>>
>>
>> On Mon, Feb 3, 2020 at 8:59 AM Fred Kiefer <fredkie...@gmx.de> wrote:
>>
>>> Am 03.02.2020 um 00:53 schrieb Sergii Stoian <stoyan...@gmail.com>:
>>>
>>> On Mon, Feb 3, 2020 at 1:05 AM Fred Kiefer <fredkie...@gmx.de> wrote:
>>>
>>> I just ran a quick scan with valgrind and this did not detect any obvious
>>> wrong memory access. Looking at the code once again I see that line 4276
>>> may be wrong for certain bytesPerRow values. Here the old code that copied
>>> over line by line is safer. Maybe we could check bytesPerRow versus
>>> pixelsWide*4 and use the old code if they are not the same?
>>>
>>> Line 4276 looks like this: "xcursorImage->yhot = hotp.y;" Do you mean
>>> memcpy call at 4279?
>>
>> Yes, it was line 4276 in the original merge commit, but has changed since
>> then.
>>
>> Could you please explain why old code is safer?
>>
>> Old code:
>> for (row = 0; row < h; row++)
>> {
>> memcpy((char*)xcursorImage->pixels + (row * (w * 4)),
>> data + (row * bytesPerRow),
>> bytesPerRow);
>> }
>>
>> New code:
>>
>> memcpy((char*)xcursorImage->pixels, data, w * h * colors);
>
>
> In general it is safer as the new code expects that the image is fully
> packed. (You moved the comment with the conversion from unpacked to packed
> over to the swap function) If bytesPerRow is not equal to w * 4 (there may be
> a few extra bytes to align stuff a bit), then the new code would not transfer
> the correct data. We would end up with random garbage in between. But in
> this special case the image comes from GSStandardImage and at least for the
> case where there are alpha values that function should already return a
> packed image. Thinking about it the old code should only have copied w * 4
> bytes for each row. The old code could have written a few bytes past the
> pixels array.
Clear explanation, thank you. It is a usual case when image contains more bytes
than image size (width * height)?
I feel we need to return back old code with your explanation in comment.
What do you think?
Sergii
