On Mon, Aug 24, 2009 at 05:45:15PM -0700, Joe Auricchio wrote: > On Sun, Aug 23, 2009 at 06:07, Robert Millan<r...@aybabtu.com> wrote: > > On Wed, Jul 22, 2009 at 10:42:54AM -0700, Joe Auricchio wrote: > >>> > >>> 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. > > > > Hi, > > > > Note that the initial patch was committed due to lack of coordination > > (there's > > been a period in which we've been sloppy due to lack of leadership, but this > > won't happen again). > > > > Nevertheless, the initial approach is probably not correct; my impression > > is that it should be using grub_zalloc() instead. If you're still > > interested, > > I'd appreciate if you could fix that. > > Using zalloc only makes sense in grub_virtual_screen_setup, when the > buffer is allocated. But the clear_chars loop is used thrice: in > _setup, in grub_virtual_screen_cls, and in scroll_up. > > Freeing and allocating a new buffer for each cls or scroll is clearly > insane. So the zalloc can't be in the common function. > > The goal of this refactor was to centralize the code for visually > clearing the text buffer. So it doesn't make sense to have _setup use > zalloc but have _cls and scroll_up use a loop without zalloc: then > what has been refactored? > > It seemed like a nice idea to use zalloc, but this code isn't the > right nail for that hammer. :) > > Let me know how you want to proceed. If you'd still like to use zalloc > I can adjust the patch.
No. When I suggested zalloc I hadn't looked at this in detail (and I still haven't). I was under the impression that you considered zalloc was suitable. If that's not the case, then there's nothing more to say. Thanks for your explanation! -- 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." _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
