Re: [edk2-devel] [PATCH v10 0/6] jansson edk2 port

["Abner Chang" <abner.chang@hpe.com>]

> -----Original Message-----
> From: Kinney, Michael D [mailto:michael.d.kinney@intel.com]
> Sent: Thursday, December 24, 2020 7:46 AM
> To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist)
> <abner.chang@hpe.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>; Andrew Fish <afish@apple.com>; Laszlo
> Ersek <lersek@redhat.com>; Leif Lindholm <leif@nuviainc.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Wang, Nickle (HPS SW)
> <nickle.wang@hpe.com>; O'Hanley, Peter (EXL) <peter.ohanley@hpe.com>
> Subject: RE: [edk2-devel] [PATCH v10 0/6] jansson edk2 port
> 
> Hi Abner,
> 
> 1) This latest version declares JsonLib as private in the DEC file.  I do not think
> you intended that.
Sign. ok
> 
> 2) In JasonLib.inf, your [BuildOptions] CC_FLAGS are different than my
> experiments found.
> 
>    MSFT:*_*_*_CC_FLAGS = /wd4204 /wd4244 /wd4090  /wd4334
> /DHAVE_CONFIG_H=1 /U_WIN32 /UWIN64 /U_MSC_VER
> 
>    It is always best to minimize all warning disables for each supported
> architecture.
> 
>    Which Visual Studio compilers have you tested with?  Do some require
> more warning disables than others?
I think you tried it on IA32? So X64 arch requires /wd4204 /wd4244 /wd4090  /wd4334 and IA32 doesn't need /wd4334. I will have separate build options for each.
> 
> 3) Where did the implementation of the string functions in RedfishCrtLib.c
> come from.  I do not
>    see any attribution or license statements.
I think you were saying the string functions such as strtol, strtoll and etc. It has been a while, this was came with Redfish POC code v4 released by Intel in 2018 to HPE. However those are from edk2-lib as fa as I can tell.
I can say something in the file header.
> 
> 4) RedfishCrtLib.h - The following 2 #defines do not work for %s.  %s is used
> with these functions
>    in jansson lib.  AsciiSPrint and AsciiVSPrint treat %s as Unicode and snprintf
> and vnsprintf
>    treat %s as ASCII.
> 
> #define snprintf(buf,len,...)             AsciiSPrint(buf,len,__VA_ARGS__)
> #define vsnprintf(buf,len,format,marker)
> AsciiVSPrint((buf),(len),(format),(marker))
> 
> I tried the following bit of sample code linked against RedfishCrtLib
> 
>   CHAR8  Message[100];
> 
>   Print (L"Hello World\n");
> 
>   snprintf (Message, sizeof(Message), "Hello %s\n", "World");
>   AsciiPrint (Message);
Ah, you were talking about the missing characters last time. jansson only uses (v)snprintf for the error and dump messages but not using on the JSON object operation. And libredfish doesn’t use these two functions fortunately.
Do you think we should parse the formatting string and replace %s with %a? Or any other suggestions?

Abner
> 
> 
> It output:
> 
>     Hello World
>     Hello Wrd
> 
> Best regards,
> 
> Mike
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Abner
> > Chang
> > Sent: Wednesday, December 23, 2020 12:11 AM
> > To: devel@edk2.groups.io
> > Cc: Sean Brogan <sean.brogan@microsoft.com>; Bret Barkelew
> > <Bret.Barkelew@microsoft.com>; Andrew Fish <afish@apple.com>; Laszlo
> > Ersek <lersek@redhat.com>; Leif Lindholm <leif@nuviainc.com>; Kinney,
> > Michael D <michael.d.kinney@intel.com>; Liming Gao
> > <gaoliming@byosoft.com.cn>; Nickle Wang <nickle.wang@hpe.com>;
> Peter
> > O'Hanley <peter.ohanley@hpe.com>
> > Subject: [edk2-devel] [PATCH v10 0/6] jansson edk2 port
> >
> > In v10, - Rename CrtLib to RedfishCrtLib
> >         - Review JsonLib /wd build options for Visual Studio.
> > In v9, - Address comments given by Mike Kinney on 12/21 In v8, -
> > Assigne patch file order
> >        - Add Acked-by tags
> > In v7, - Remove C RTC header files to under [Include.Common.Private]
> >          in RedfishPkg.dec.
> >        - address comments given by Mike Kinney.
> > In v6, Remove JanssonJsonMapping.h
> > In v5, move BaseUcs2Utf8Lib to under RedfishPkg.
> > In v4,
> >        - Address review comments
> >        - Seperate CRT functions to a individule library CrtLib under
> >          RedfishPkg.
> >        - Seperate UCS2-UTF8 functions to a individule library
> >          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 (not
> > contributed yet) and the edk2 port of libredfish library (not
> > contributed yet) based on DMTF GitHub
> > (https://github.com/DMTF/libredfish).
> >
> > Jansson is licensed under the MIT license(refer to ReadMe.rst under edk2).
> > It is used in production and its API is stable. In UEFI/EDKII
> > environment, Redfish project consumes jansson to achieve JSON
> operations.
> >
> > * Jansson version on edk2: 2.13.1
> >
> > * EDKII jansson library wrapper:
> >    - JsonLib.h:
> >      This is the denifitions of EDKII JSON APIs which are mapped to
> >      jannson funcitons accordingly.
> >
> >    - JanssonJsonLibMapping.h:
> >      This is the wrapper file to map funcitons and definitions used in
> >      native jannson applications to edk2 JsonLib. This avoids the
> >      modifications on native jannson applications to be built under
> >      edk2 environment.
> >
> > *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 macro.
> >   The PR is submitted to jansson open source community.
> >   https://github.com/akheron/jansson/pull/558
> >
> > Signed-off-by: Abner Chang <abner.chang@hpe.com>
> >
> > Cc: Sean Brogan <sean.brogan@microsoft.com>
> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > Cc: Andrew Fish <afish@apple.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Leif Lindholm <leif@nuviainc.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Nickle Wang <nickle.wang@hpe.com>
> > Cc: Peter O'Hanley <peter.ohanley@hpe.com>
> >
> > 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/Crt/assert.h               |   16 +
> >  RedfishPkg/Include/Crt/errno.h                |   16 +
> >  RedfishPkg/Include/Crt/limits.h               |   16 +
> >  RedfishPkg/Include/Crt/math.h                 |   16 +
> >  RedfishPkg/Include/Crt/stdarg.h               |   15 +
> >  RedfishPkg/Include/Crt/stddef.h               |   16 +
> >  RedfishPkg/Include/Crt/stdio.h                |   15 +
> >  RedfishPkg/Include/Crt/stdlib.h               |   16 +
> >  RedfishPkg/Include/Crt/string.h               |   16 +
> >  RedfishPkg/Include/Crt/sys/time.h             |   15 +
> >  RedfishPkg/Include/Crt/sys/types.h            |   15 +
> >  RedfishPkg/Include/Crt/time.h                 |   15 +
> >  RedfishPkg/Include/Library/BaseUcs2Utf8Lib.h  |   61 +
> >  RedfishPkg/Include/Library/CrtLib.h           |  191 +++
> >  RedfishPkg/Include/Library/JsonLib.h          |  763 +++++++++++
> >  .../Library/BaseUcs2Utf8Lib/BaseUcs2Utf8Lib.c |  421 +++++++
> >  .../BaseUcs2Utf8Lib/BaseUcs2Utf8Lib.inf       |   31 +
> >  RedfishPkg/Library/CrtLib/CrtLib.c            |  595 +++++++++
> >  RedfishPkg/Library/CrtLib/CrtLib.inf          |   38 +
> >  RedfishPkg/Library/JsonLib/JsonLib.c          |  964 ++++++++++++++
> >  RedfishPkg/Library/JsonLib/JsonLib.inf        |   89 ++
> >  RedfishPkg/Library/JsonLib/Readme.rst         |   40 +
> >  RedfishPkg/Library/JsonLib/jansson            |    1 +
> >  RedfishPkg/Library/JsonLib/jansson_config.h   |   41 +
> >  .../Library/JsonLib/jansson_private_config.h  |   19 +
> >  RedfishPkg/Library/JsonLib/load.c             | 1111 +++++++++++++++++
> >  RedfishPkg/RedfishLibs.dsc.inc                |    3 +
> >  RedfishPkg/RedfishPkg.ci.yaml                 |   25 +
> >  RedfishPkg/RedfishPkg.dec                     |   25 +
> >  RedfishPkg/RedfishPkg.dsc                     |    3 +
> >  33 files changed, 4614 insertions(+)
> >  create mode 100644 RedfishPkg/Include/Crt/assert.h  create mode
> > 100644 RedfishPkg/Include/Crt/errno.h  create mode 100644
> > RedfishPkg/Include/Crt/limits.h  create mode 100644
> > RedfishPkg/Include/Crt/math.h  create mode 100644
> > RedfishPkg/Include/Crt/stdarg.h  create mode 100644
> > RedfishPkg/Include/Crt/stddef.h  create mode 100644
> > RedfishPkg/Include/Crt/stdio.h  create mode 100644
> > RedfishPkg/Include/Crt/stdlib.h  create mode 100644
> > RedfishPkg/Include/Crt/string.h  create mode 100644
> > RedfishPkg/Include/Crt/sys/time.h  create mode 100644
> > RedfishPkg/Include/Crt/sys/types.h
> >  create mode 100644 RedfishPkg/Include/Crt/time.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 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/load.c
> >
> > --
> > 2.17.1
> >
> >
> >
> > 
> >