From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1MZu0T-0006mS-Lf for mharc-grub-devel@gnu.org; Sat, 08 Aug 2009 18:03:17 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MZu0R-0006jT-TO for grub-devel@gnu.org; Sat, 08 Aug 2009 18:03:15 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MZu0R-0006iQ-71 for grub-devel@gnu.org; Sat, 08 Aug 2009 18:03:15 -0400 Received: from [199.232.76.173] (port=35723 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MZu0R-0006iI-3j for grub-devel@gnu.org; Sat, 08 Aug 2009 18:03:15 -0400 Received: from fg-out-1718.google.com ([72.14.220.159]:16542) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MZu0Q-0007o3-Fh for grub-devel@gnu.org; Sat, 08 Aug 2009 18:03:14 -0400 Received: by fg-out-1718.google.com with SMTP id 22so526105fge.7 for ; Sat, 08 Aug 2009 15:03:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:received:in-reply-to :references:date:x-google-sender-auth:message-id:subject:from:to :content-type:content-transfer-encoding; bh=0ZdJFEz+05R48Cs2nKV4Etp8Y55rZ4yDj2Hq4UIG9Tc=; b=UPRAKsHxA3KkiiBdOw7UOOFeOaAVN7zoxYbF83BDUhNtaUUrDVcrs25cVIzlplPpIp yCCc1luqjcN4Izcm/AZZtvlegCxn1jI6XsEnLVtZ8axWOA1ZG9zCkIINkRbKxq6qhKSn dxhmk6NP4DSyZkfi4YpmrnD2zeWYdep5dPDx4= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:content-type :content-transfer-encoding; b=EgMNZ+ZJHEfIGOoUQrJrTA5UOLABA3IWpXnqoB8rnYpcFFVgqg/59MgfpHRkkXZuoa lYtZNUKigtUYpPd772AP0UZTxlRRD5DuPXakCZlB95vvVOuGHofgtfuMJhs6jOueQ2Iu /Vt98ZDYpff8/OocCWUKXgd3ubrAI1yyhxhkk= MIME-Version: 1.0 Sender: hramrach@gmail.com Received: by 10.86.3.5 with SMTP id 5mr2060909fgc.41.1249768993554; Sat, 08 Aug 2009 15:03:13 -0700 (PDT) In-Reply-To: References: <1248392495.2661.208.camel@mj> <20090801150118.GB23133@thorin> Date: Sun, 9 Aug 2009 00:03:13 +0200 X-Google-Sender-Auth: 4cf3c08325950f2c Message-ID: From: Michal Suchanek To: The development of GRUB 2 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by monty-python.gnu.org: GNU/Linux 2.6 (newer, 2) Subject: Re: Fwd: [PATCH 1/2] Framebuffer split X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GRUB 2 List-Id: The development of GRUB 2 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 08 Aug 2009 22:03:16 -0000 2009/8/8 Michal Suchanek : > Hello > > I cannot get any sense of these patches "with moving code around > omitted" so I tried the git repository in Vladimir's signature. > > Maybe I am missing something but the function of this code escapes me: > > static grub_err_t > grub_video_vbe_set_viewport (unsigned int x, unsigned int y, > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0unsigned int width, unsigned int height) > { > =C2=A0/* Make sure viewport is withing screen dimensions. =C2=A0If viewpo= rt was set > =C2=A0 =C2=A0 to be out of the region, mark its size as zero. =C2=A0*/ > =C2=A0if (x > active_mode_info.x_resolution) > =C2=A0 =C2=A0{ > =C2=A0 =C2=A0 =C2=A0x =3D 0; > =C2=A0 =C2=A0 =C2=A0width =3D 0; > =C2=A0 =C2=A0} > > =C2=A0if (y > active_mode_info.y_resolution) > =C2=A0 =C2=A0{ > =C2=A0 =C2=A0 =C2=A0y =3D 0; > =C2=A0 =C2=A0 =C2=A0height =3D 0; > =C2=A0 =C2=A0} > > =C2=A0if (x + width > active_mode_info.x_resolution) > =C2=A0 =C2=A0width =3D active_mode_info.x_resolution - x; > > =C2=A0if (y + height > active_mode_info.y_resolution) > =C2=A0 =C2=A0height =3D active_mode_info.y_resolution - y; > =C2=A0return grub_video_fb_set_viewport (x, y, width, height); > } > > As I understand it the code checks the bounds against the active > videomode and then sets the viewport on the active render target. > Since the active render target can be arbitrarily set by the user (to, > say an offscreen bitmap for rendering the terminal text) I do not see > how these two parts match. > It seems that this check should be done in grub_video_fb_set_viewport instead, and this function completely eliminated until viewport on non-active target can be set. The assumption about bounds being checked beforehand could break for the blit* functions in fbblit.c because the video_fb blit dispatcher only checks viewport on the target, and viewport is not checked against mode when set. There is get_data_ptr in fbutil.c and grub_video_fb_get_video_ptr in video_fb.c that seem to do the same thing. They do not have the warning abut bounds checking which they do not do. This warning is also missing from get/set_pixel. Perhaps there should also be get/set_pixel method for video adapters? The functions in fbutil.c probably should not be used from outside fb this library but there are no user counterparts in video_fb.c Thanks Michal