From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1f4Pmy-0003dx-7F for mharc-grub-devel@gnu.org; Fri, 06 Apr 2018 07:44:16 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51498) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f4Pmv-0003dR-NO for grub-devel@gnu.org; Fri, 06 Apr 2018 07:44:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1f4Pmq-0006AX-NL for grub-devel@gnu.org; Fri, 06 Apr 2018 07:44:13 -0400 Received: from userp2130.oracle.com ([156.151.31.86]:60858) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1f4Pmq-00069Q-Ee for grub-devel@gnu.org; Fri, 06 Apr 2018 07:44:08 -0400 Received: from pps.filterd (userp2130.oracle.com [127.0.0.1]) by userp2130.oracle.com (8.16.0.22/8.16.0.22) with SMTP id w36B49kP106269; Fri, 6 Apr 2018 11:44:06 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=date : from : to : cc : subject : message-id : references : mime-version : content-type : in-reply-to; s=corp-2017-10-26; bh=K+62bKwUwCvdwtNtaFWvzlR452QR+Je+/67im10bkjQ=; b=m0u+3PPzSfRIyt4Sk/Bu5hcF4/Sx+R16x4tUILVEQAxLPt9Xk373DwZQIGnBgP+Y4b0P nRrad4e9loTMCL4BaAKqUMKz5Du8PnaBriRiX91uLFIoV914JRC6e7bjcMuZEYKoU1I8 DqjHmLEBptlHhgh2dfsvyO/8zEmnvJ7K9A/X7vulsVocDmZAxc2i5X4q5NexkWmwkvSd O8M+4n4tM3coPKBxR26aeQizANfOwXuId4qQrZY/nXSz7ll2Ywt78xgRkGNgUcGJRB11 sVlVgQiTbPVZlZYJqb2AlnEkcoK6YdEsbL4Zws4KOfkjIzwHP7ESwrLDOYLRE0KlT0Mu ng== Received: from aserv0022.oracle.com (aserv0022.oracle.com [141.146.126.234]) by userp2130.oracle.com with ESMTP id 2h5k57vpcu-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 06 Apr 2018 11:44:06 +0000 Received: from userv0122.oracle.com (userv0122.oracle.com [156.151.31.75]) by aserv0022.oracle.com (8.14.4/8.14.4) with ESMTP id w36Bi54Z014590 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 6 Apr 2018 11:44:05 GMT Received: from abhmp0015.oracle.com (abhmp0015.oracle.com [141.146.116.21]) by userv0122.oracle.com (8.14.4/8.14.4) with ESMTP id w36Bi5MI011479; Fri, 6 Apr 2018 11:44:05 GMT Received: from olila.local.net-space.pl (/10.175.209.68) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Fri, 06 Apr 2018 04:44:04 -0700 Date: Fri, 6 Apr 2018 13:44:01 +0200 From: Daniel Kiper To: Hans de Goede Cc: grub-devel@gnu.org Subject: Re: [PATCH 4/4] EFI: Do not set text-mode until we actually need it Message-ID: <20180406114401.GX26100@olila.local.net-space.pl> References: <20180328145028.21555-1-hdegoede@redhat.com> <20180328145028.21555-5-hdegoede@redhat.com> <20180405123401.GU26100@olila.local.net-space.pl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-Proofpoint-Virus-Version: vendor=nai engine=5900 definitions=8854 signatures=668697 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 mlxscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1711220000 definitions=main-1804060118 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [generic] [fuzzy] X-Received-From: 156.151.31.86 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Apr 2018 11:44:15 -0000 On Thu, Apr 05, 2018 at 08:05:47PM +0200, Hans de Goede wrote: > Hi, > > On 05-04-18 14:34, Daniel Kiper wrote: > >On Wed, Mar 28, 2018 at 04:50:28PM +0200, Hans de Goede wrote: > >>If we're running with a hidden menu we may never need text mode, so do not > >>change the video-mode to text until we actually need it. > >> > >>Signed-off-by: Hans de Goede > >>--- > >> grub-core/term/efi/console.c | 72 +++++++++++++++++++++++------------- > >> 1 file changed, 47 insertions(+), 25 deletions(-) > >> > >>diff --git a/grub-core/term/efi/console.c b/grub-core/term/efi/console.c > >>index 02f64ea74..d9fd7cf48 100644 > >>--- a/grub-core/term/efi/console.c > >>+++ b/grub-core/term/efi/console.c > >>@@ -24,6 +24,11 @@ > >> #include > >> #include > >> > >>+static grub_err_t grub_prepare_for_text_output(struct grub_term_output *term); > > > >Please drop this forward declaration and put the function definition before the callers. > > I did not do that initially because that requires also moving > grub_console_setcolorstate() and grub_console_setcursor() to > higher in the file (or do a forward declaration for those). > > I'll add a preparation patch to v2 of the series moving just > those 2 up to higher in the file. Great! > >>+static int text_mode_available = -1; > > > >Could you use bool type here? If yes please define grub_bool_t as stdbool.h > >does (grub/bool.h?) and replace its usage in grub-core/term/tparm.c as separate > >patch. If not bool please use enum here. > > There are 3 states, so a bool is not going to work I will > switch to an enum for v2. Perfect! > >>+static int text_colorstate = -1; > > There already is a grub_term_color_state enum for this, which > defines values 0-2, I want to know if setcolorstate has been > called and text_colorstate contains a valid value, so I made > this an int and use -1 to mean not set / invalid. > > I can see a number of solutions here: > > 1) Leave as is > 2) Use: > > static grub_term_color_state text_colorstate = -1; > > Assuming the compiler will not warn about putting -1 in there. > > 3) Extend the grub_term_color_state like this: > > typedef enum > { > /* Used for uninitialized grub_term_color_state variables */ > GRUB_TERM_COLOR_UNDEFINED = -1, > /* The color used to display all text that does not use the > user defined colors below. */ > GRUB_TERM_COLOR_STANDARD = 0, > /* The user defined colors for normal text. */ > GRUB_TERM_COLOR_NORMAL, > /* The user defined colors for highlighted text. */ > GRUB_TERM_COLOR_HIGHLIGHT > } > grub_term_color_state; > > Which solution would you prefer? 3rd, enum please. Thanks, Daniel