[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: crash when using local display but not remote
From: |
Fred Kiefer |
Subject: |
Re: crash when using local display but not remote |
Date: |
Tue, 4 Feb 2020 19:43:15 +0100 |
> Am 04.02.2020 um 11:21 schrieb Sergii Stoian <address@hidden>:
>
>
> On Mon, Feb 3, 2020 at 8:59 AM Fred Kiefer <address@hidden> wrote:
>
> > Am 03.02.2020 um 00:53 schrieb Sergii Stoian <address@hidden>:
> >
> > On Mon, Feb 3, 2020 at 1:05 AM Fred Kiefer <address@hidden> 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.
- Re: crash when using local display but not remote, Riccardo Mottola, 2020/02/02
- Re: crash when using local display but not remote, Riccardo Mottola, 2020/02/02
- Re: crash when using local display but not remote, Fred Kiefer, 2020/02/02
- Re: crash when using local display but not remote, Sergii Stoian, 2020/02/02
- Re: crash when using local display but not remote, Fred Kiefer, 2020/02/03
- Re: crash when using local display but not remote, Sergii Stoian, 2020/02/04
- Re: crash when using local display but not remote,
Fred Kiefer <=
- Re: crash when using local display but not remote, Sergii Stoian, 2020/02/04
- Re: crash when using local display but not remote, Fred Kiefer, 2020/02/07
- Re: crash when using local display but not remote, Riccardo Mottola, 2020/02/08
- Re: crash when using local display but not remote, Fred Kiefer, 2020/02/08
- Re: crash when using local display but not remote, Riccardo Mottola, 2020/02/17
- Re: crash when using local display but not remote, Fred Kiefer, 2020/02/18
- Re: crash when using local display but not remote, Riccardo Mottola, 2020/02/18
- Re: crash when using local display but not remote, Fred Kiefer, 2020/02/18
- Re: crash when using local display but not remote, Riccardo Mottola, 2020/02/18
- Re: crash when using local display but not remote, Fred Kiefer, 2020/02/21