grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Clear out gfxterm's virtual text_buffer - fixes junk at end


From: Robert Millan
Subject: Re: [PATCH] Clear out gfxterm's virtual text_buffer - fixes junk at end of lines
Date: Wed, 22 Jul 2009 20:04:10 +0200
User-agent: Mutt/1.5.18 (2008-05-17)

On Wed, Jul 22, 2009 at 10:42:54AM -0700, Joe Auricchio wrote:
>
> On 22 Jul, 2009, at 10:27 , Robert Millan wrote:
>
>> On Sat, Jul 18, 2009 at 03:39:00PM -0700, Joe Auricchio wrote:
>>>
>>> On 18 Jul, 2009, at 11:58 , Robert Millan wrote:
>>>> On Sat, Jul 18, 2009 at 12:15:04AM -0700, Joe Auricchio wrote:
>>>>>
>>>>> This fixes a 100% reproducible and very annoying bug I've found:
>>>>> Switch
>>>>> to gfxterm and until your clear the screen or scroll, the last
>>>>> position
>>>>> of every line has a random junk character in random foreground and
>>>>> background color. I believe the root cause is that the
>>>>> virtual_screen's
>>>>> text_buffer is uninitialized; as the cursor reaches a new position,
>>>>> the
>>>>> uninitialized junk data is drawn to the screen. Really, the entire
>>>>> screen
>>>>> is junk, but it's never marked dirty so it's never drawn. So let's
>>>>> just
>>>>> clear out the whole text_buffer when we set up the screen.
>>>>
>>>> But grub_virtual_screen_free() already set the whole structure to  
>>>> 0 at
>>>> the
>>>> beginning of grub_virtual_screen_setup().
>>>>
>>>> Does a zero-filled screen result in garbage, or is something else
>>>> overwriting
>>>> it?
>>>
>>> grub_virtual_screen_free sets all of virtual_screen to 0, but not
>>> virtual_screen.text_buffer. text_buffer just gets grub_freed. Later  
>>> in
>>> grub_virtual_screen_setup, a new text_buffer is grub_malloced. It's  
>>> this
>>> new text_buffer that my patch clears out.
>>
>> So perhaps this can be solved simpler by replacing grub_malloc with
>> grub_zalloc ?
>
> I saw the zalloc commit and thought the same thing! :D I'll test it. But 
> fg_color and bg_color must be set to the screen's current colors, which 
> may not be zero.
>
> I just discovered that
> - this patch to grub_virtual_screen_setup()
> - scroll_up() line 579 /* Clear last line in text buffer. */, and
> - grub_virtual_screen_cls()
> do the same thing.
>
> An opportunity to refactor? I'll look into this.

Please do.  We're very interested in code cleanup.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."




reply via email to

[Prev in Thread] Current Thread [Next in Thread]