Re: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS spec 3.1.1

[Tim Lewis <tim.lewis@insyde.com>]

Jaben --

I don't know exactly what you mean by each "compiler of the shell". I do know that HII strings and HII string handling add the largest discretionary, non-spec-required overhead to the Shell executable (based on our production mini-shell vs. the EDK2 shell) even without any second language.  I understand it as a design goal of the EDK2 shell to allow localization (although it does not actually do any itself). I also understand that it is easier to enforce the strings-in-uni rule as an absolute. But it does come with a cost for those who want to use a fully functional shell in a much more restricted environment.


Tim

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Carsey, Jaben
Sent: Monday, January 23, 2017 1:15 PM
To: Tim Lewis <tim.lewis@insyde.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: Carsey, Jaben <jaben.carsey@intel.com>
Subject: Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS spec 3.1.1

Tim,

I agree that some strings are directly from SMBIOS or UEFI specifications and are not needed to be localized.  

Is there a disadvantage to allowing each compiler of the shell to make that an individual decision?

In either case, it might be worthwhile to verify that all of the embedded strings meet that criterion.

-Jaben

> -----Original Message-----
> From: Tim Lewis [mailto:tim.lewis@insyde.com]
> Sent: Monday, January 23, 2017 12:52 PM
> To: Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ruiyu 
> <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; edk2- 
> devel@lists.01.org
> Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS 
> spec 3.1.1
> Importance: High
> 
> Also, in some cases, the text is taken directly from the specification.
> Introducing HII strings in order to make these translatable when the 
> source material is normative doesn't help, IMO.
> 
> Tim
> 
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of 
> Carsey, Jaben
> Sent: Monday, January 23, 2017 9:41 AM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; 
> edk2- devel@lists.01.org
> Cc: Carsey, Jaben <jaben.carsey@intel.com>
> Subject: Re: [edk2] [PATCH 3/3] ShellPkg SmbiosView: Add decoding of 
> SMBIOS spec 3.1.1
> 
> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> 
> I think that string mixed use existed in the EDK version of the 
> command and was just never removed.
> 
> -Jaben
> 
> > -----Original Message-----
> > From: Ni, Ruiyu
> > Sent: Sunday, January 22, 2017 1:49 AM
> > To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
> > Cc: Carsey, Jaben <jaben.carsey@intel.com>
> > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS 
> > spec 3.1.1
> > Importance: High
> >
> > Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
> >
> > Thanks/Ray
> >
> > > -----Original Message-----
> > > From: Zeng, Star
> > > Sent: Sunday, January 22, 2017 5:25 PM
> > > To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> > > Cc: Carsey, Jaben <jaben.carsey@intel.com>; Zeng, Star 
> > > <star.zeng@intel.com>
> > > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of 
> > > SMBIOS spec 3.1.1
> > >
> > > Ray & Jaben,
> > >
> > > I am not so sure about the rule.
> > > The mixed using of strings in c code and strings in uni file has 
> > > been there since it was created.
> > >
> > >
> > > Thanks,
> > > Star
> > > -----Original Message-----
> > > From: Ni, Ruiyu
> > > Sent: Sunday, January 22, 2017 4:56 PM
> > > To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
> > > Cc: Carsey, Jaben <jaben.carsey@intel.com>
> > > Subject: RE: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of 
> > > SMBIOS spec 3.1.1
> > >
> > > Star,
> > > Why some strings are hardcoded in C file while some are defined in 
> > > UNI
> > file?
> > > What's the rule?
> > >
> > > Thanks/Ray
> > >
> > > > -----Original Message-----
> > > > From: Zeng, Star
> > > > Sent: Sunday, January 22, 2017 4:18 PM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Zeng, Star <star.zeng@intel.com>; Ni, Ruiyu 
> > > > <ruiyu.ni@intel.com>; Carsey, Jaben <jaben.carsey@intel.com>
> > > > Subject: [PATCH 3/3] ShellPkg SmbiosView: Add decoding of SMBIOS
> > spec
> > > > 3.1.1
> > > >
> > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=349
> > > >
> > > > Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> > > > Cc: Jaben Carsey <jaben.carsey@intel.com>
> > > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > > Signed-off-by: Star Zeng <star.zeng@intel.com>
> > > > ---
> > > >  .../SmbiosView/PrintInfo.c                         |  6 +++-
> > > >  .../SmbiosView/QueryTable.c                        | 37
> > ++++++++++++++++++++++
> > > >  .../SmbiosView/QueryTable.h                        | 14 +++++++-
> > > >  .../SmbiosView/SmbiosViewStrings.uni               |  3 +-
> > > >  4 files changed, 57 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git
> > > >
> > a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > > >
> > b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > > > index ecb8e2492453..1d6002b92593 100644
> > > > ---
> > > >
> > a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > > > +++
> > > >
> > b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > > > @@ -1106,7 +1106,7 @@ SmbiosPrintStructure (
> > > >    // Management Controller Host Interface (Type 42)
> > > >    //
> > > >    case 42:
> > > > -    PRINT_STRUCT_VALUE_H (Struct, Type42, InterfaceType);
> > > > +    DisplayMCHostInterfaceType (Struct->Type42->InterfaceType, 
> > > > + Option);
> > > >      break;
> > > >
> > > >    //
> > > > @@ -1818,6 +1818,10 @@ DisplayProcessorFamily (
> > > >      Print (L"AMD Opteron(TM) X3000 Series APU\n");
> > > >      break;
> > > >
> > > > +  case 0x6B:
> > > > +    Print (L"AMD Zen Processor Family\n");
> > > > +    break;
> > > > +
> > > >    case 0x70:
> > > >      ShellPrintHiiEx(-1,-1,NULL,STRING_TOKEN
> > > > (STR_SMBIOSVIEW_PRINTINFO_HOBBIT_FAMILY),
> > gShellDebug1HiiHandle);
> > > >      break;
> > > > diff --git
> > > >
> > >
> >
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.
> > > > c
> > > >
> > >
> >
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.
> > > > c
> > > > index 4a06c12e3b2b..282ba584c8c9 100644
> > > > ---
> > > >
> > >
> >
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.
> > > > c
> > > > +++
> > > >
> > >
> >
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.
> > > > +++ c
> > > > @@ -575,6 +575,10 @@ TABLE_ITEM  ProcessorUpgradeTable[] = {
> > > >    {
> > > >      0x37,
> > > >      L"Socket SP3"
> > > > +  },
> > > > +  {
> > > > +    0x38,
> > > > +    L"Socket SP3r2"
> > > >    }
> > > >  };
> > > >
> > > > @@ -3156,6 +3160,22 @@ TABLE_ITEM
> IPMIDIBMCInterfaceTypeTable[]
> > =
> > > {
> > > >    },
> > > >  };
> > > >
> > > > +TABLE_ITEM  MCHostInterfaceTypeTable[] = {
> > > > +  {
> > > > +    0x3F00,
> > > > +    L" MCTP Host Interface "
> > > > +  },
> > > > +  {
> > > > +    0x40,
> > > > +    L" Network Host Interface "
> > > > +  },
> > > > +  {
> > > > +    0xF0,
> > > > +    L" OEM defined "
> > > > +  },
> > > > +};
> > > > +
> > > > +
> > > >  TABLE_ITEM  StructureTypeInfoTable[] = {
> > > >    {
> > > >      0,
> > > > @@ -4525,6 +4545,23 @@ DisplayIPMIDIBMCInterfaceType (  }
> > > >
> > > >  /**
> > > > +  Display Management Controller Host Interface (Type 42) information.
> > > > +
> > > > +  @param[in] Key      The key of the structure.
> > > > +  @param[in] Option   The optional information.
> > > > +**/
> > > > +VOID
> > > > +DisplayMCHostInterfaceType (
> > > > +  IN UINT8 Key,
> > > > +  IN UINT8 Option
> > > > +  )
> > > > +{
> > > > +  ShellPrintHiiEx(-1,-1,NULL,STRING_TOKEN
> > > > +(STR_SMBIOSVIEW_QUERYTABLE_MC_HOST_INTERFACE_TYPE),
> > > > +gShellDebug1HiiHandle);
> > > > +  PRINT_INFO_OPTION (Key, Option);
> > > > +  PRINT_TABLE_ITEM (MCHostInterfaceTypeTable, Key); }
> > > > +
> > > > +/**
> > > >    Display the structure type information.
> > > >
> > > >    @param[in] Key      The key of the structure.
> > > > diff --git
> > > >
> > >
> >
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.
> > > > h
> > > >
> > >
> >
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.
> > > > h
> > > > index 9cae7094fb19..bd9e6898d40a 100644
> > > > ---
> > > >
> > >
> >
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.
> > > > h
> > > > +++
> > > >
> > >
> >
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.
> > > > +++ h
> > > > @@ -2,7 +2,7 @@
> > > >    Build a table, each item is (key, info) pair.
> > > >    and give a interface of query a string out of a table.
> > > >
> > > > -  Copyright (c) 2005 - 2012, Intel Corporation. All rights 
> > > > reserved.<BR>
> > > > +  Copyright (c) 2005 - 2017, Intel Corporation. All rights 
> > > > + reserved.<BR>
> > > >    This program and the accompanying materials
> > > >    are licensed and made available under the terms and 
> > > > conditions of the BSD License
> > > >    which accompanies this distribution.  The full text of the 
> > > > license may be found at @@ -779,4 +779,16 @@
> > DisplayIPMIDIBMCInterfaceType (
> > > >    IN UINT8 Option
> > > >    );
> > > >
> > > > +/**
> > > > +  Display Management Controller Host Interface (Type 42) information.
> > > > +
> > > > +  @param[in] Key      The key of the structure.
> > > > +  @param[in] Option   The optional information.
> > > > +**/
> > > > +VOID
> > > > +DisplayMCHostInterfaceType (
> > > > +  IN UINT8 Key,
> > > > +  IN UINT8 Option
> > > > +  );
> > > > +
> > > >  #endif
> > > > diff --git
> > > >
> > >
> >
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosView
> > > > Strings.uni
> > > >
> > >
> >
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosView
> > > > Strings.uni
> > > > index b9032df076d2..7d694536dbcd 100644
> > > > ---
> > > >
> > >
> >
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosView
> > > > Strings.uni
> > > > +++
> > > >
> > >
> >
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosView
> > > > S
> > > > +++ trings.uni
> > > > @@ -1,6 +1,6 @@
> > > >  // /**
> > > >  //
> > > > -// Copyright (c) 2005 - 2015, Intel Corporation. All rights 
> > > > reserved.<BR>
> > > > +// Copyright (c) 2005 - 2017, Intel Corporation. All rights 
> > > > +reserved.<BR>
> > > >  // (C) Copyright 2014-2015 Hewlett-Packard Development Company, 
> > > > L.P.<BR> // (C) Copyright 2015-2017 Hewlett Packard Enterprise 
> > > > Development LP<BR> // This program and the accompanying 
> > > > materials
> > @@
> > > > -444,6 +444,7 @@  #string
> > > > STR_SMBIOSVIEW_QUERYTABLE_MANAGEMENT_DEV_ADDR_TYPE
> > > > #language en-US "Management Device - Address Type:"
> > > >  #string STR_SMBIOSVIEW_QUERYTABLE_MEM_CHANNEL_TYPE
> > > > #language en-US "Memory Channel Type:"
> > > >  #string STR_SMBIOSVIEW_QUERYTABLE_BMC_INTERFACE_TYPE
> > > > #language en-US "BMC Interface Type:"
> > > > +#string STR_SMBIOSVIEW_QUERYTABLE_MC_HOST_INTERFACE_TYPE
> > > > #language en-US "MC Host Interface Type:"
> > > >  #string STR_SMBIOSVIEW_QUERYTABLE_STRUCT_TYPE
> > #language
> > > > en-US "Structure Type:"
> > > >  #string STR_SMBIOSVIEW_SMBIOSVIEW_ONE_VAR_ARGV
> > > > #language en-US "%s "
> > > >  #string STR_SMBIOSVIEW_SMBIOSVIEW_QUERY_STRUCT_COND
> > > > #language en-US "Query Structure, conditions are:\r\n"
> > > > --
> > > > 2.7.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel