GNU bug report logs - #37756
[PATCH] Wrong initialization of fringe bitmap

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 37756 in the body.
You can then email your comments to 37756 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Wrong initialization of fringe bitmap
Date: Mon, 14 Oct 2019 23:39:19 -0300

[Message part 1 (text/plain, inline)]

In fringe.c:1606 you have:

  xfb = xmalloc (sizeof fb + fb.height * BYTES_PER_BITMAP_ROW);
  fb.bits = b = ((unsigned short *)
ptr_bounds_clip (xfb + 1, fb.height * BYTES_PER_BITMAP_ROW));
  xfb = ptr_bounds_clip (xfb, sizeof *xfb);
  memset (b, 0, fb.height);

I might be wrong but it seems to me that the last line should be:

  memset (b, 0, fb.height * BYTES_PER_BITMAP_ROW);

instead.

I've attached a patch that does exactly that.

Best regards
--
Carlos

[0001-Fix-zero-initialization-of-fringe-bitmap.patch (text/x-patch, attachment)]

Message #8 received at 37756 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Carlos Pita <carlosjosepita <at> gmail.com>
Cc: 37756 <at> debbugs.gnu.org
Subject: Re: bug#37756: [PATCH] Wrong initialization of fringe bitmap
Date: Tue, 15 Oct 2019 12:47:26 +0300

> From: Carlos Pita <carlosjosepita <at> gmail.com>
> Date: Mon, 14 Oct 2019 23:39:19 -0300
> 
> In fringe.c:1606 you have:
> 
>   xfb = xmalloc (sizeof fb + fb.height * BYTES_PER_BITMAP_ROW);
>   fb.bits = b = ((unsigned short *)
> ptr_bounds_clip (xfb + 1, fb.height * BYTES_PER_BITMAP_ROW));
>   xfb = ptr_bounds_clip (xfb, sizeof *xfb);
>   memset (b, 0, fb.height);
> 
> I might be wrong but it seems to me that the last line should be:
> 
>   memset (b, 0, fb.height * BYTES_PER_BITMAP_ROW);
> 
> instead.

Can you explain your reasoning?  The loop after that does initialize
the structure as needed, doesn't it?

Message #11 received at 37756 <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 37756 <at> debbugs.gnu.org
Subject: Re: bug#37756: [PATCH] Wrong initialization of fringe bitmap
Date: Tue, 15 Oct 2019 07:14:25 -0300

> Can you explain your reasoning?  The loop after that does initialize

Ok, TBH I hadn't looked at the loop. But anyway:

> >   xfb = xmalloc (sizeof fb + fb.height * BYTES_PER_BITMAP_ROW);

This allocates fb.height * BYTES_PER_BITMAP_ROW for bits = b.

> >   memset (b, 0, fb.height);

This only initializes fb.height bytes to zero.

And then the loop indeed initializes all fb.height *
BYTES_PER_BITMAP_ROW bytes (b being an unsigned short pointer):

  j = 0;
  while (j < fb.height)
    {
       ...
       b[j++] = something;
       ...
    }

So instead of memset (b, 0, fb.height) being incomplete I would now
say that it is redundant and misleading.

Message #14 received at 37756 <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 37756 <at> debbugs.gnu.org
Subject: Re: bug#37756: [PATCH] Wrong initialization of fringe bitmap
Date: Wed, 16 Oct 2019 14:28:29 -0300

[Message part 1 (text/plain, inline)]

Here is a patch that removes the superfluous line with a detailed
explanation in the commit message.

I've tested it in combination (also in combination with my "hidpi
fringe" patches) and got no segv yet ;)

[0001-Remove-redundant-and-incomplete-initialization-of-fr.patch (application/x-patch, attachment)]

Message #17 received at 37756 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Carlos Pita <carlosjosepita <at> gmail.com>
Cc: 37756 <at> debbugs.gnu.org
Subject: Re: bug#37756: [PATCH] Wrong initialization of fringe bitmap
Date: Wed, 16 Oct 2019 21:35:03 +0300

> From: Carlos Pita <carlosjosepita <at> gmail.com>
> Date: Wed, 16 Oct 2019 14:28:29 -0300
> Cc: 37756 <at> debbugs.gnu.org
> 
> Here is a patch that removes the superfluous line with a detailed
> explanation in the commit message.

Thanks, but please format the log message according to CONTRIBUTE: it
should name the function in which the change is being made, start with
a capital letter, and keep 2 spaces between sentences.  Also, there's
no need to describe in such detail what the code does, when the code
speaks for itself.  You could just say "Remove redundant zeroing of
fb.bits".

Message #20 received at 37756 <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 37756 <at> debbugs.gnu.org
Subject: Re: bug#37756: [PATCH] Wrong initialization of fringe bitmap
Date: Wed, 16 Oct 2019 15:54:01 -0300

[Message part 1 (text/plain, inline)]

I've done as you asked, Eli. Sorry but I'm still trying to catch up
with many of your conventions.

I'm not sure about how to designate the modified function in the
changelog, since it's a DEFUN.

[0001-Remove-redundant-and-incomplete-initialization-of-fr.patch (text/x-patch, attachment)]

Message #23 received at 37756 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Carlos Pita <carlosjosepita <at> gmail.com>
Cc: 37756 <at> debbugs.gnu.org
Subject: Re: bug#37756: [PATCH] Wrong initialization of fringe bitmap
Date: Wed, 16 Oct 2019 22:05:57 +0300

> From: Carlos Pita <carlosjosepita <at> gmail.com>
> Date: Wed, 16 Oct 2019 15:54:01 -0300
> Cc: 37756 <at> debbugs.gnu.org
> 
> I've done as you asked, Eli. Sorry but I'm still trying to catch up
> with many of your conventions.

Thanks for persevering.

> I'm not sure about how to designate the modified function in the
> changelog, since it's a DEFUN.

Use one of the related Emacs commands, and Emacs will do that for you.
Those commands are described in CONTRIBUTE under "Generating ChangeLog
entries".

Message #26 received at 37756 <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 37756 <at> debbugs.gnu.org
Subject: Re: bug#37756: [PATCH] Wrong initialization of fringe bitmap
Date: Wed, 16 Oct 2019 17:01:11 -0300

[Message part 1 (text/plain, inline)]

> Use one of the related Emacs commands, and Emacs will do that for you.
> Those commands are described in CONTRIBUTE under "Generating ChangeLog
> entries".

I'm having some issues doing that because I'm using magit to amend /
reword previously committed changes, so the instructions there don't
quite match my situation. Will try harder later, for now I just
modified the message after some examples in your git log. Hope it's ok
now.

[0001-Remove-redundant-initialization-of-fringe-bitmap-Bug.patch (text/x-patch, attachment)]

Message #29 received at 37756 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Carlos Pita <carlosjosepita <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 37756 <at> debbugs.gnu.org
Subject: Re: bug#37756: [PATCH] Wrong initialization of fringe bitmap
Date: Thu, 17 Oct 2019 12:53:51 +0100

On Wed, Oct 16, 2019 at 05:01:11PM -0300, Carlos Pita wrote:
> > Use one of the related Emacs commands, and Emacs will do that for you.
> > Those commands are described in CONTRIBUTE under "Generating ChangeLog
> > entries".
> 
> I'm having some issues doing that because I'm using magit to amend /
> reword previously committed changes, so the instructions there don't
> quite match my situation. Will try harder later, for now I just
> modified the message after some examples in your git log. Hope it's ok
> now.

In the Magit diff view that is displayed when comitting you should be
able to just hit ‘C’ with point in the code you want to add to the
commit message.
-- 
Alan Third

Message #32 received at 37756 <at> debbugs.gnu.org (full text, mbox):

From: Robert Pluim <rpluim <at> gmail.com>
To: Alan Third <alan <at> idiocy.org>
Cc: Carlos Pita <carlosjosepita <at> gmail.com>, 37756 <at> debbugs.gnu.org
Subject: Re: bug#37756: [PATCH] Wrong initialization of fringe bitmap
Date: Thu, 17 Oct 2019 17:01:54 +0200

>>>>> On Thu, 17 Oct 2019 12:53:51 +0100, Alan Third <alan <at> idiocy.org> said:

    Alan> On Wed, Oct 16, 2019 at 05:01:11PM -0300, Carlos Pita wrote:
    >> > Use one of the related Emacs commands, and Emacs will do that for you.
    >> > Those commands are described in CONTRIBUTE under "Generating ChangeLog
    >> > entries".
    >> 
    >> I'm having some issues doing that because I'm using magit to amend /
    >> reword previously committed changes, so the instructions there don't
    >> quite match my situation. Will try harder later, for now I just
    >> modified the message after some examples in your git log. Hope it's ok
    >> now.

    Alan> In the Magit diff view that is displayed when comitting you should be
    Alan> able to just hit ‘C’ with point in the code you want to add to the
    Alan> commit message.

That doesnʼt auto-snarf the function names the way C-x 4 A or C-c C-w
do in vc.

Robert

Message #37 received at 37756-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Carlos Pita <carlosjosepita <at> gmail.com>
Cc: 37756-done <at> debbugs.gnu.org
Subject: Re: bug#37756: [PATCH] Wrong initialization of fringe bitmap
Date: Sat, 26 Oct 2019 13:21:25 +0300

> From: Carlos Pita <carlosjosepita <at> gmail.com>
> Date: Wed, 16 Oct 2019 17:01:11 -0300
> Cc: 37756 <at> debbugs.gnu.org
> 
> I'm having some issues doing that because I'm using magit to amend /
> reword previously committed changes, so the instructions there don't
> quite match my situation. Will try harder later, for now I just
> modified the message after some examples in your git log. Hope it's ok
> now.

Thanks, pushed.  Closing the bug.

> From: memeplex <carlosjosepita <at> gmail.com>
        ^^^^^^^^
Is this really how you want your name to appear in the Git log?

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.