From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 7D16281C1C for ; Thu, 12 Jan 2017 07:39:50 -0800 (PST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga103.jf.intel.com with ESMTP; 12 Jan 2017 07:39:49 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,219,1477983600"; d="scan'208,217";a="808078435" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by FMSMGA003.fm.intel.com with ESMTP; 12 Jan 2017 07:39:49 -0800 Received: from fmsmsx155.amr.corp.intel.com (10.18.116.71) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 12 Jan 2017 07:39:48 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by FMSMSX155.amr.corp.intel.com (10.18.116.71) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 12 Jan 2017 07:39:47 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.59]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.204]) with mapi id 14.03.0248.002; Thu, 12 Jan 2017 23:39:44 +0800 From: "Ni, Ruiyu" To: Laszlo Ersek , "edk2-devel@ml01.01.org" CC: "Tian, Feng" , "Zeng, Star" Thread-Topic: [edk2] [PATCH 4/8] MdeModulePkg/TerminalDxe: Refine InitializeTerminalConsoleTextMode Thread-Index: AQHSax0Zg3z8fbE8PE+Lvryxm7Hr/6E0JJeAgADZEsA= Date: Thu, 12 Jan 2017 15:39:43 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5B87CAE9@SHSMSX104.ccr.corp.intel.com> References: <20170110083904.34104-1-ruiyu.ni@intel.com> <20170110083904.34104-5-ruiyu.ni@intel.com> <32be930e-69f2-0a7e-7641-b57a464e3092@redhat.com> In-Reply-To: <32be930e-69f2-0a7e-7641-b57a464e3092@redhat.com> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiOGUxMThmYzgtNGY4NC00OGYyLWFlNDYtYTM0NTkwMWFjMDkzIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6InFkY083M0hWRkR6ZXhqaEJxRHVqZmJ4OFJrK0VUQ3d2Mkg0Mkd6RFVFK1k9In0= x-ctpclassification: CTP_IC x-originating-ip: [10.239.127.40] MIME-Version: 1.0 X-Content-Filtered-By: Mailman/MimeDel 2.1.21 Subject: Re: [PATCH 4/8] MdeModulePkg/TerminalDxe: Refine InitializeTerminalConsoleTextMode X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Jan 2017 15:39:50 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Laszlo, Thanks for your reply. I will remove the comments in C function. Regards, Ray From: Laszlo Ersek [mailto:lersek@redhat.com] Sent: Thursday, January 12, 2017 6:42 PM To: Ni, Ruiyu ; edk2-devel@ml01.01.org Cc: Tian, Feng ; Zeng, Star Subject: Re: [edk2] [PATCH 4/8] MdeModulePkg/TerminalDxe: Refine Initialize= TerminalConsoleTextMode Ray, sorry for the late followup, I just noticed a (trivial) omission in the patch: On 01/10/17 09:39, Ruiyu Ni wrote: > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ruiyu Ni > > Cc: Star Zeng > > Cc: Feng Tian > > --- > .../Universal/Console/TerminalDxe/Terminal.c | 108 +++++----------= ------ > 1 file changed, 26 insertions(+), 82 deletions(-) > > diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c b/MdeM= odulePkg/Universal/Console/TerminalDxe/Terminal.c > index f1ce12d..4b0c00d 100644 > --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c > +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c > @@ -113,6 +113,8 @@ TERMINAL_DEV mTerminalDevTemplate =3D { > }; > > TERMINAL_CONSOLE_MODE_DATA mTerminalConsoleModeData[] =3D { > + {80, 25}, > + {80, 50}, > {100, 31}, > // > // New modes can be added here. > @@ -450,97 +452,39 @@ TerminalFreeNotifyList ( > It returns information for available text modes that the terminal can = support. > > @param[out] TextModeCount The total number of text modes that ter= minal console supports. > - @param[out] TextModeData The buffer to the text modes column and= row information. > - Caller is responsible to free it when i= t's non-NULL. > > - @retval EFI_SUCCESS The supporting mode information is retu= rned. > - @retval EFI_INVALID_PARAMETER The parameters are invalid. > + @return The buffer to the text modes column and row information. > + Caller is responsible to free it when it's non-NULL. > > **/ > -EFI_STATUS > +TERMINAL_CONSOLE_MODE_DATA * > InitializeTerminalConsoleTextMode ( > - OUT UINTN *TextModeCount, > - OUT TERMINAL_CONSOLE_MODE_DATA **TextModeData > - ) > + OUT INT32 *TextModeCount > +) > { > - UINTN Index; > - UINTN Count; > - TERMINAL_CONSOLE_MODE_DATA *ModeBuffer; > - TERMINAL_CONSOLE_MODE_DATA *NewModeBuffer; > - UINTN ValidCount; > - UINTN ValidIndex; > - > - if ((TextModeCount =3D=3D NULL) || (TextModeData =3D=3D NULL)) { > - return EFI_INVALID_PARAMETER; > - } > - > - Count =3D sizeof (mTerminalConsoleModeData) / sizeof (TERMINAL_CONSOLE= _MODE_DATA); > - > - // > - // Get defined mode buffer pointer. > - // > - ModeBuffer =3D mTerminalConsoleModeData; > - > + TERMINAL_CONSOLE_MODE_DATA *TextModeData; > + > + ASSERT (TextModeCount !=3D NULL); > + > // > // Here we make sure that the final mode exposed does not include the = duplicated modes, > // and does not include the invalid modes which exceed the max column = and row. > // Reserve 2 modes for 80x25, 80x50 of terminal console. > // This comment too should have been removed, given that no reservation occurs any longer; the 80x25 and 80x50 modes have been moved to "mTerminalConsoleModeData", and are copied like all the other modes in that array, with AllocateCopyPool(). Thanks Laszlo > - NewModeBuffer =3D AllocateZeroPool (sizeof (TERMINAL_CONSOLE_MODE_DATA= ) * (Count + 2)); > - ASSERT (NewModeBuffer !=3D NULL); > - > - // > - // Mode 0 and mode 1 is for 80x25, 80x50 according to UEFI spec. > - // > - ValidCount =3D 0; > - > - NewModeBuffer[ValidCount].Columns =3D 80; > - NewModeBuffer[ValidCount].Rows =3D 25; > - ValidCount++; > - > - NewModeBuffer[ValidCount].Columns =3D 80; > - NewModeBuffer[ValidCount].Rows =3D 50; > - ValidCount++; > - > - // > - // Start from mode 2 to put the valid mode other than 80x25 and 80x50 = in the output mode buffer. > - // > - for (Index =3D 0; Index < Count; Index++) { > - if ((ModeBuffer[Index].Columns =3D=3D 0) || (ModeBuffer[Index].Rows = =3D=3D 0)) { > - // > - // Skip the pre-defined mode which is invalid. > - // > - continue; > - } > - for (ValidIndex =3D 0; ValidIndex < ValidCount; ValidIndex++) { > - if ((ModeBuffer[Index].Columns =3D=3D NewModeBuffer[ValidIndex].Co= lumns) && > - (ModeBuffer[Index].Rows =3D=3D NewModeBuffer[ValidIndex].Rows)= ) { > - // > - // Skip the duplicated mode. > - // > - break; > - } > - } > - if (ValidIndex =3D=3D ValidCount) { > - NewModeBuffer[ValidCount].Columns =3D ModeBuffer[Index].Columns; > - NewModeBuffer[ValidCount].Rows =3D ModeBuffer[Index].Rows; > - ValidCount++; > - } > + TextModeData =3D AllocateCopyPool (sizeof (mTerminalConsoleModeData), = mTerminalConsoleModeData); > + if (TextModeData =3D=3D NULL) { > + return NULL; > } > - > + *TextModeCount =3D ARRAY_SIZE (mTerminalConsoleModeData); > + > DEBUG_CODE ( > - for (Index =3D 0; Index < ValidCount; Index++) { > - DEBUG ((EFI_D_INFO, "Terminal - Mode %d, Column =3D %d, Row =3D %d= \n", > - Index, NewModeBuffer[Index].Columns, NewModeB= uffer[Index].Rows)); > + INT32 Index; > + for (Index =3D 0; Index < *TextModeCount; Index++) { > + DEBUG ((DEBUG_INFO, "Terminal - Mode %d, Column =3D %d, Row =3D %d= \n", > + Index, TextModeData[Index].Columns, TextModeData[Index].Ro= ws)); > } > ); > - > - // > - // Return valid mode count and mode information buffer. > - // > - *TextModeCount =3D ValidCount; > - *TextModeData =3D NewModeBuffer; > - return EFI_SUCCESS; > + return TextModeData; > } > > /** > @@ -632,7 +576,6 @@ TerminalDriverBindingStart ( > BOOLEAN SimTxtInInstalled; > BOOLEAN SimTxtOutInstalled; > BOOLEAN FirstEnter; > - UINTN ModeCount; > > TerminalDevice =3D NULL; > ConInSelected =3D FALSE; > @@ -896,12 +839,13 @@ TerminalDriverBindingStart ( > ); > SimpleTextOutput->Mode =3D &TerminalDevice->SimpleTextOutputMode; > > - Status =3D InitializeTerminalConsoleTextMode (&ModeCount, &TerminalD= evice->TerminalConsoleModeData); > - if (EFI_ERROR (Status)) { > + TerminalDevice->TerminalConsoleModeData =3D InitializeTerminalConsol= eTextMode ( > + &SimpleTextOutput->Mode-= >MaxMode > + ); > + if (TerminalDevice->TerminalConsoleModeData =3D=3D NULL) { > goto ReportError; > } > - TerminalDevice->SimpleTextOutputMode.MaxMode =3D (INT32) ModeCount; > - > + > // > // For terminal devices, cursor is always visible > // >