From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by mx.groups.io with SMTP id smtpd.web09.28501.1607909384440844105 for ; Sun, 13 Dec 2020 17:29:46 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: byosoft.com.cn, ip: 58.240.74.242, mailfrom: gaoliming@byosoft.com.cn) Received: from DESKTOPS6D0PVI ([58.246.60.130]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Mon, 14 Dec 2020 09:29:37 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: , , Cc: "'Sean Brogan'" , "'Bret Barkelew'" , "'Andrew Fish'" , "'Laszlo Ersek'" , "'Leif Lindholm'" , "'Wang, Nickle \(HPS SW\)'" , "'O'Hanley, Peter \(EXL\)'" References: <20201208021048.9104-1-abner.chang@hpe.com> <008a01d6cd2c$eb0a1550$c11e3ff0$@byosoft.com.cn> In-Reply-To: Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0gW1BBVENIIHY1IDAvNl0gamFuc3NvbiBlZGsyIHBvcnQ=?= Date: Mon, 14 Dec 2020 09:29:37 +0800 Message-ID: <004701d6d1b8$9669c4b0$c33d4e10$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQKkLOZuLovEk1vLn9DUkBPmPDW97wLEqjBGAUxSeKoCbVnR4QJ33A2xAgymVngCGfYMO6fyMBZQ Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Abner: > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > =E5=8F=91=E4=BB=B6=E4=BA=BA: bounce+27952+68741+4905953+8761045@groups.i= o > =E4=BB=A3=E8=A1=A8 Abner = Chang > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2020=E5=B9=B412=E6=9C=8813=E6=97= =A5 12:02 > =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io; michael.d.kinney@inte= l.com; > gaoliming@byosoft.com.cn > =E6=8A=84=E9=80=81: 'Sean Brogan' ; 'Bret Bar= kelew' > ; 'Andrew Fish' ; 'Laszlo > Ersek' ; 'Leif Lindholm' ; Wang, > Nickle (HPS SW) ; O'Hanley, Peter (EXL) > > =E4=B8=BB=E9=A2=98: Re: [edk2-devel] [PATCH v5 0/6] jansson edk2 port >=20 >=20 >=20 > > -----Original Message----- > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > > Michael D Kinney > > Sent: Saturday, December 12, 2020 3:23 AM > > To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist) > > ; gaoliming@byosoft.com.cn; Kinney, Michael D > > > > Cc: 'Sean Brogan' ; 'Bret Barkelew' > > ; 'Andrew Fish' ; > 'Laszlo > > Ersek' ; 'Leif Lindholm' ; Wang, > > Nickle (HPS SW) ; O'Hanley, Peter (EXL) > > > > Subject: Re: [edk2-devel] [PATCH v5 0/6] jansson edk2 port > > > > Abner, > > > > Feedback below. > > > > Mike > > > > > -----Original Message----- > > > From: devel@edk2.groups.io On Behalf Of Abner > > > Chang > > > Sent: Wednesday, December 9, 2020 8:02 PM > > > To: Kinney, Michael D ; > > > devel@edk2.groups.io; gaoliming@byosoft.com.cn > > > Cc: 'Sean Brogan' ; 'Bret Barkelew' > > ; 'Andrew Fish' > > > ; 'Laszlo Ersek' ; 'Leif Lindhol= m' > > > ; Wang, Nickle (HPS SW) ; > > > O'Hanley, Peter (EXL) > > > Subject: Re: [edk2-devel] [PATCH v5 0/6] jansson edk2 port > > > > > > > > > > > > > -----Original Message----- > > > > From: Kinney, Michael D [mailto:michael.d.kinney@intel.com] > > > > Sent: Thursday, December 10, 2020 10:33 AM > > > > To: Chang, Abner (HPS SW/FW Technologist) ; > > > > devel@edk2.groups.io; gaoliming@byosoft.com.cn; Kinney, Michael D > > > > > > > > Cc: 'Sean Brogan' ; 'Bret Barkelew' > > > > ; 'Andrew Fish' ; > > > > 'Laszlo Ersek' ; 'Leif Lindholm' > > > > ; Wang, Nickle (HPS SW) ; > > > > O'Hanley, Peter (EXL) > > > > Subject: RE: [edk2-devel] [PATCH v5 0/6] jansson edk2 port > > > > > > > > Abner, > > > > > > > > Some questions included below. > > > > > > > > Mike > > > > > > > > > > > > > -----Original Message----- > > > > > From: Chang, Abner (HPS SW/FW Technologist) > > > > > > > Sent: Wednesday, December 9, 2020 6:14 PM > > > > > To: devel@edk2.groups.io; gaoliming@byosoft.com.cn > > > > > Cc: 'Sean Brogan' ; 'Bret Barkelew' > > > > ; 'Andrew Fish' > > > > > ; 'Laszlo Ersek' ; 'Leif > > Lindholm' > > > > > ; Kinney, Michael D > > > > > ; Wang, Nickle (HPS SW) > > > > > ; O'Hanley, Peter (EXL) > > > > > > > > > > Subject: RE: [edk2-devel] [PATCH v5 0/6] jansson edk2 port > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On > > > > > > Behalf Of gaoliming > > > > > > Sent: Tuesday, December 8, 2020 2:40 PM > > > > > > To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist= ) > > > > > > > > > > > > Cc: 'Sean Brogan' ; 'Bret Barkelew' > > > > > > ; 'Andrew Fish' > ; > > > > > > 'Laszlo Ersek' ; 'Leif Lindholm' > > > > > > ; 'Michael D Kinney' > > > > > > ; Wang, Nickle (HPS SW) > > > > > > ; O'Hanley, Peter (EXL) > > > > > > > > > > Subject: =E5=9B=9E=E5=A4=8D: [edk2-devel] [PATCH v5 0/6] janss= on edk2 port > > > > > > > > > > > > Abner: > > > > > > I have minor comments on the library header file. > > > > > > > > > > > > 1. JasonLib.h & BaseUcs2Utf8Lib.h. They don't need to include > > > > > > the additional header files, such as Uefi.h and BaseLib.h, > > > > > > because the library header file doesn't depend on the definiti= ons > > from BaseLib. > > > > > > 2. CrtLib.inf needs to list the required library class: > > > > > > BaseMemoryLib & > > > > PrintLib. > > > > > > > > > > OK, I will wait couple days for other comments and address that > > > > > all > > > > together. > > > > > > > > > > Thanks > > > > > > > > > > > > Thanks > > > > > > Liming > > > > > > > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > > > > > > > =E5=8F=91=E4=BB=B6=E4=BA=BA: bounce+27952+68426+4905953+8761= 045@groups.io > > > > > > > =E4=BB=A3=E8= =A1=A8 > Abner > > > > Chang > > > > > > > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2020=E5=B9=B412=E6=9C= =888=E6=97=A5 10:11 > > > > > > > =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io > > > > > > > =E6=8A=84=E9=80=81: Sean Brogan ;= Bret Barkelew > > > > > > > ; Andrew Fish > ; > > > > > > > Laszlo Ersek ; Leif Lindholm > > > > > > > ; Michael D Kinney > > > > > > > ; Liming Gao > > > > > > > ; Nickle Wang > > ; > > > > > > Peter > > > > > > > O'Hanley > > > > > > > =E4=B8=BB=E9=A2=98: [edk2-devel] [PATCH v5 0/6] jansson edk2= port > > > > > > > > > > > > > > In v5, move BaseUcs2Utf8Lib to under RedfishPkg. > > > > > > > In v4, > > > > > > > - Address review comments > > > > > > > - Seperate CRT functions to a individule library CrtL= ib > under > > > > > > > RedfishPkg. > > > > > > > - Seperate UCS2-UTF8 functions to a individule librar= y > > > > > > > BaseUcs2Utf8Lib under MdeModulePkg. > > > > > > > > > > > > > > In v3, Add jansson library as the required submoudle in > > > > > > > CiSettings.py for CI test. > > > > > > > In v2, JsonLib is moved to under RedfishPkg. > > > > > > > > > > > > > > edk2 JSON library is based on jansson open source > > > > > > > (https://github.com/akheron/jansson) and wrapped as an edk2 > > library. > > > > > > > edk2 JsonLib will be used by edk2 Redfish feature drivers (n= ot > > > > > > > contributed yet) and the edk2 port of libredfish library (no= t > > > > > > > contributed yet) based on DMTF GitHub > > > > > > > (https://github.com/DMTF/libredfish). > > > > > > > > > > > > > > Jansson is licensed under the MIT license(refer to ReadMe.rs= t > > > > > > > under > > > > edk2). > > > > > > > It is used in production and its API is stable. In UEFI/EDKI= I > > > > > > > environment, Redfish project consumes jansson to achieve JSO= N > > > > > > operations. > > > > > > > > > > > > > > * Jansson version on edk2: 2.13.1 > > > > > > > > > > > > > > * EDKII jansson library wrapper: > > > > > > > - JsonLib.h: > > > > > > > This is the denifitions of EDKII JSON APIs which are ma= pped > to > > > > > > > jannson funcitons accordingly. > > > > > > > > > > > > > > - JanssonJsonLibMapping.h: > > > > > > > This is the wrapper file to map funcitons and definitio= ns > used in > > > > > > > native jannson applications to edk2 JsonLib. This avoid= s the > > > > > > > modifications on native jannson applications to be buil= t > under > > > > > > > edk2 environment. > > > > > > > > Can you explain the use case for this in more detail? > > > > What are these native jannson applications and why do we need to > > > > build them in edk2? > > > > If we have the jannson submodule, why can't these apps just use th= e > > > > standard jannson services? > > > > > > > > I ask because this is a new concept for edk2 and I want to make su= re > > > > it is really required. > > > > > > The users of jannson library are LibRedfish open source > > > (https://github.com/DMTF/libredfish) and the Edk2 Redfish features > > drivers (WIP) to communicate with Redfish service. EDK2 port of Libred= fish > > will be sent for review after this one. > > > Libredfish use "jansson.h" in its source code and uses the native > > > jansson functions directly. I think most of open source projects use= the > > same way to leverage jansson open source project. > > > However, "jansson.h" is defined in the source code directory that ed= k2 > > module can't refer to it in edk2 metafiles. > > > > Why not? The package DEC file can list include paths. > Mike, I thought we are recommended to not referring to the header file i= n the > source code or the private header files in the module. My memory may out= of > date but I remember that the package's "Include/" directory should conta= ins > all public headers files that are exposed to other packages or this pack= age. Is > this still valid in the edk2 driver writer's guide? I can't find any sim= ilar > sentence which restrict to use the header files out of "Include/" direct= ory in > UEFI driver writer's guide. > But any way, that's why we use a wrapper header file and put it under > Include/. >=20 This is true for the normal module. But, submodule code is the different.= =20 Submodule code is in module directory. To access its header file, its dire= ctory is=20 Required to be specified in [Includes] section of Package.dec file.=20 Thanks Liming > > > > For example, the RedFishPkg DEC file currently has the following [Incl= udes] > > section: > > > > [Includes] > > Include > > > > I could be updated to: > > > > [Includes] > > Include > > Library\JsonLib\jansson\src > > > > This would allow libs/modules that want to directly use the jansson se= rvices > > to use > > > > #include > > > > If you wanted to limit the use of to only libs/modules in t= he > > RedfishPkg, then you could use the private include feature: > > > > [Includes] > > Include > > > > [Includes.Common.Private] > > Library\JsonLib\jansson\src > > > > In fact, this exact pattern is used in the UnitTestFrameworkPkg to all= ow > > comonponents to use the standard includes from the cmocka submodule: > > > > [Includes] > > Library/CmockaLib/cmocka/include > > > > [Includes.Common.Private] > > PrivateInclude > > Library/CmockaLib/cmocka/include/cmockery > > > > > > Please evaluate this approach and see of the JassonJsoonMapping.h file= can > > be removed. > > > > > Thus we need an wrapper for these jansson applications. That > > > JanssonJsonMapping.h defines the EDK2 style API for mapping native > > > jansson functions. For those edk2 based JSON applications, they can = just > > use edk2 style APIs. Such as the edk2 Redfish feature drivers, those c= an use > > the EDK2 coding style compliant API to invoke jansson functions by usi= ng > > JsonLib.h. > > > For those native jansson applications, we can just use > > > JanssonJsonMapping.h to map the native jansson API to EDK2 style API > > > provided by JsonLib.lib. JanssonJsonMapping.h is not just map the > > > functions, it also gives edk2 style prototype for jansson variables = such as > > json_t. These edk2 style prototypes are used in EDK2 Redfish drivers o= r > other > > edk2 based application as well. > > > > > > > > > > > > > > > > > > > > > *Known issue: > > > > > > > Build fail with jansson/src/load.c, overrride and add code= in > load.c > > > > > > > to conditionally use stdin according to HAVE_UNISTD_H macr= o. > > > > > > > The PR is submitted to jansson open source community. > > > > > > > https://github.com/akheron/jansson/pull/558 > > > > > > > > > > > > > > Signed-off-by: Abner Chang > > > > > > > > > > > > > > Cc: Sean Brogan > > > > > > > Cc: Bret Barkelew > > > > > > > Cc: Andrew Fish > > > > > > > Cc: Laszlo Ersek > > > > > > > Cc: Leif Lindholm > > > > > > > Cc: Michael D Kinney > > > > > > > Cc: Liming Gao > > > > > > > Cc: Nickle Wang > > > > > > > Cc: Peter O'Hanley > > > > > > > > > > > > > > Abner Chang (6): > > > > > > > RedfishPkg/Ucs2Utf8lib: UCS2 to UFT8 manipulation library > > > > > > > edk2: jansson submodule for edk2 JSON library > > > > > > > RedfishPkg/CrtLib: C runtime library > > > > > > > RedfishPkg/library: EDK2 port of jansson library > > > > > > > RedfishPkg: Add EDK2 port of jansson library to build > > > > > > > .pytool: Add required submodule for JsonLib > > > > > > > > > > > > > > .gitmodules | 3 + > > > > > > > .pytool/CISettings.py | 2 + > > > > > > > ReadMe.rst | 1 > + > > > > > > > RedfishPkg/Include/JanssonJsonMapping.h | 63 + > > > > > > > RedfishPkg/Include/Library/BaseUcs2Utf8Lib.h | 69 + > > > > > > > RedfishPkg/Include/Library/CrtLib.h | 195 +++ > > > > > > > RedfishPkg/Include/Library/JsonLib.h | 768 > ++++++++++++ > > > > > > > .../Library/BaseUcs2Utf8Lib/BaseUcs2Utf8Lib.c | 417 > +++++++ > > > > > > > .../BaseUcs2Utf8Lib/BaseUcs2Utf8Lib.inf | 31 + > > > > > > > RedfishPkg/Library/CrtLib/CrtLib.c | 705 > +++++++++++ > > > > > > > RedfishPkg/Library/CrtLib/CrtLib.inf | 37 + > > > > > > > RedfishPkg/Library/JsonLib/JsonLib.c | 961 > ++++++++++++++ > > > > > > > RedfishPkg/Library/JsonLib/JsonLib.inf | 101 ++ > > > > > > > RedfishPkg/Library/JsonLib/Readme.rst | 40 + > > > > > > > RedfishPkg/Library/JsonLib/assert.h | 16 + > > > > > > > RedfishPkg/Library/JsonLib/errno.h | 16 + > > > > > > > RedfishPkg/Library/JsonLib/jansson | 1 + > > > > > > > RedfishPkg/Library/JsonLib/jansson_config.h | 46 + > > > > > > > .../Library/JsonLib/jansson_private_config.h | 19 + > > > > > > > RedfishPkg/Library/JsonLib/limits.h | 16 + > > > > > > > RedfishPkg/Library/JsonLib/load.c | 1111 > > > > > > > +++++++++++++++++ > > > > > > > RedfishPkg/Library/JsonLib/math.h | 16 + > > > > > > > RedfishPkg/Library/JsonLib/stdarg.h | 15 + > > > > > > > RedfishPkg/Library/JsonLib/stddef.h | 16 + > > > > > > > RedfishPkg/Library/JsonLib/stdio.h | 15 + > > > > > > > RedfishPkg/Library/JsonLib/stdlib.h | 16 + > > > > > > > RedfishPkg/Library/JsonLib/string.h | 16 + > > > > > > > RedfishPkg/Library/JsonLib/sys/time.h | 15 + > > > > > > > RedfishPkg/Library/JsonLib/sys/types.h | 15 + > > > > > > > RedfishPkg/Library/JsonLib/time.h | 15 + > > > > > > > RedfishPkg/RedfishLibs.dsc.inc | 3 + > > > > > > > RedfishPkg/RedfishPkg.ci.yaml | 33 + > > > > > > > RedfishPkg/RedfishPkg.dec | 15 + > > > > > > > RedfishPkg/RedfishPkg.dsc | 3 + > > > > > > > 34 files changed, 4811 insertions(+) create mode 100644 > > > > > > > RedfishPkg/Include/JanssonJsonMapping.h > > > > > > > create mode 100644 > > > > > > > RedfishPkg/Include/Library/BaseUcs2Utf8Lib.h > > > > > > > create mode 100644 RedfishPkg/Include/Library/CrtLib.h > > > > > > > create mode 100644 RedfishPkg/Include/Library/JsonLib.h > > > > > > > create mode 100644 > > > > > > > RedfishPkg/Library/BaseUcs2Utf8Lib/BaseUcs2Utf8Lib.c > > > > > > > create mode 100644 > > > > > > > RedfishPkg/Library/BaseUcs2Utf8Lib/BaseUcs2Utf8Lib.inf > > > > > > > create mode 100644 RedfishPkg/Library/CrtLib/CrtLib.c > > > > > > > create mode 100644 RedfishPkg/Library/CrtLib/CrtLib.inf > > > > > > > create mode 100644 RedfishPkg/Library/JsonLib/JsonLib.c > > > > > > > create mode 100644 RedfishPkg/Library/JsonLib/JsonLib.inf > > > > > > > create mode 100644 RedfishPkg/Library/JsonLib/Readme.rst > > > > > > > create mode 100644 RedfishPkg/Library/JsonLib/assert.h > > > > > > > create mode 100644 RedfishPkg/Library/JsonLib/errno.h > > > > > > > create mode 160000 RedfishPkg/Library/JsonLib/jansson > > > > > > > create mode 100644 > > > > > > > RedfishPkg/Library/JsonLib/jansson_config.h > > > > > > > create mode 100644 > > > > > > > RedfishPkg/Library/JsonLib/jansson_private_config.h > > > > > > > create mode 100644 RedfishPkg/Library/JsonLib/limits.h > > > > > > > create mode 100644 RedfishPkg/Library/JsonLib/load.c creat= e > > > > > > > mode > > > > > > > 100644 RedfishPkg/Library/JsonLib/math.h create mode 100644 > > > > > > > RedfishPkg/Library/JsonLib/stdarg.h > > > > > > > create mode 100644 RedfishPkg/Library/JsonLib/stddef.h > > > > > > > create mode 100644 RedfishPkg/Library/JsonLib/stdio.h > > > > > > > create mode 100644 RedfishPkg/Library/JsonLib/stdlib.h > > > > > > > create mode 100644 RedfishPkg/Library/JsonLib/string.h > > > > > > > create mode 100644 RedfishPkg/Library/JsonLib/sys/time.h > > > > > > > create mode 100644 RedfishPkg/Library/JsonLib/sys/types.h > > > > > > > create mode 100644 RedfishPkg/Library/JsonLib/time.h > > > > > > > > > > > > > > -- > > > > > > > 2.17.1 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >=20 >=20 >=20 >=20 >=20