Hao, 
  I have some comments for three patches. Others are good to me. 

Patch: BaseTools/TianoCompress: Avoid possible NULL pointer dereference
Comment:  Please free allocated buffer after error happens. 

Patch: BaseTools/C/Common: Fix parameter format mismatch in scanf functions
Comment:  Please add more description to say why INT32 is used in sscanf.

Patch: BaseTools/VolInfo: Use hard-coded format string for calls to sprintf()
Comment: Why the format string may be changed accidentally?

Thanks
Liming
> -----Original Message-----
> From: Wu, Hao A
> Sent: Wednesday, October 12, 2016 8:20 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a...@intel.com>; Gao, Liming <liming....@intel.com>;
> Zhu, Yonghong <yonghong....@intel.com>; Dong, Eric
> <eric.d...@intel.com>; Bi, Dandan <dandan...@intel.com>
> Subject: [PATCH 00/52] Resolve issues for C source codes in BaseTools
> 
> The patch series fixes the following types of issues for C source codes in
> BaseTools:
> 
> 1. Avoid possible NULL pointer dereference
> 2. Initialize local variables before use
> 3. Remove unused local variables
> 4. Avoid accessing over array bounds
> 5. Resolve possible memory leak
> 6. Resolve file handles not being closed
> 7. Resolve possible buffer overflow in printf/scanf functions
> 
> The patch series is also available at:
> https://github.com/hwu25/edk2/tree/BaseTools_V1
> 
> Cc: Liming Gao <liming....@intel.com>
> Cc: Yonghong Zhu <yonghong....@intel.com>
> Cc: Eric Dong <eric.d...@intel.com>
> Cc: Dandan Bi <dandan...@intel.com>
> 
> Hao Wu (52):
>   BaseTools/C/Common: Avoid possible NULL pointer dereference
>   BaseTools/EfiRom: Avoid possible NULL pointer dereference
>   BaseTools/GenFfs: Avoid possible NULL pointer dereference
>   BaseTools/GenFv: Avoid possible NULL pointer dereference
>   BaseTools/GenFw: Avoid possible NULL pointer dereference
>   BaseTools/GenPage: Avoid possible NULL pointer dereference
>   BaseTools/GenSec: Avoid possible NULL pointer dereference
>   BaseTools/GenVtf: Avoid possible NULL pointer dereference
>   BaseTools/TianoCompress: Avoid possible NULL pointer dereference
>   BaseTools/VfrCompile: Avoid possible NULL pointer dereference
>   BaseTools/VolInfo: Avoid possible NULL pointer dereference
>   BaseTools/TianoCompress: Initialize local variables before being used
>   BaseTools/VfrCompile: Initialize local variables before being used
>   BaseTools/GenBootSector: Fix parameter format mismatch in printf
>     functions
>   BaseTools/VolInfo: Fix parameter format mismatch in printf functions
>   BaseTools/C/Common: Fix parameter format mismatch in scanf functions
>   BaseTools/GenFv: Fix parameter format mismatch in scanf functions
>   BaseTools/GenFw: Fix parameter format mismatch in scanf functions
>   BaseTools/GenVtf: Fix parameter format mismatch in scanf functions
>   BaseTools/C/Common: Fix potential access over array bounds
>   BaseTools/EfiRom: Fix potential access over array bounds
>   BaseTools/GenFv: Fix potential access over array bounds
>   BaseTools/TianoCompress: Fix potential access over array bounds
>   BaseTools/VfrCompile: Fix potential access over array bounds
>   BaseTools/VfrCompile: Avoid freeing memory with mismatched functions
>   BaseTools/VfrCompile: Add assignment operator definition for some
>     classes
>   BaseTools/VfrCompile: Avoid freeing freed memory in classes
>   BaseTools/VfrCompile: Remove unused local variables
>   BaseTools/C/Common: Fix potential memory leak
>   BaseTools/EfiRom: Fix potential memory leak
>   BaseTools/GenFv: Fix potential memory leak
>   BaseTools/GenPage: Fix potential memory leak
>   BaseTools/GenSec: Fix potential memory leak
>   BaseTools/GenVtf: Fix potential memory leak
>   BaseTools/Split: Fix potential memory and resource leak
>   BaseTools/TianoCompress: Fix potential memory leak
>   BaseTools/VfrCompile: Fix potential memory leak
>   BaseTools/VolInfo: Fix potential memory leak
>   BaseTools/EfiRom: Fix file handles not being closed
>   BaseTools/GenBootSector: Fix file handles not being closed
>   BaseTools/GenCrc32: Fix file handles not being closed
>   BaseTools/GenFv: Fix file handles not being closed
>   BaseTools/GenVtf: Fix file handles not being closed
>   BaseTools/LzmaCompress: Fix file handles not being closed
>   BaseTools/TianoCompress: Fix file handles not being closed
>   BaseTools/VolInfo: Fix file handles not being closed
>   BaseTools/GenVtf: Fix potential buffer overflow in scanf functions
>   BaseTools/VolInfo: Fix potential buffer overflow in scanf functions
>   BaseTools/VfrCompile: Explicitly state format string for DebugMsg()
>   BaseTools/VolInfo: Use hard-coded format string for calls to sprintf()
>   BaseTools/VfrCompile/Pccts: Add virtual destructor for class
>     DLGInputStream
>   BaseTools/VfrCompile/Pccts: Make assignment operator not returning
>     void
> 
>  BaseTools/Source/C/Common/BasePeCoff.c             |  12 ++
>  BaseTools/Source/C/Common/CommonLib.c              |   8 +-
>  BaseTools/Source/C/Common/Decompress.c             |  41 ++++--
>  BaseTools/Source/C/Common/EfiUtilityMsgs.c         |  20 +--
>  BaseTools/Source/C/Common/FirmwareVolumeBuffer.c   |   6 +-
>  BaseTools/Source/C/Common/MemoryFile.c             |   3 +-
>  BaseTools/Source/C/Common/MyAlloc.c                |  55 +++++++-
>  .../Source/C/Common/ParseGuidedSectionTools.c      |  21 ++--
>  BaseTools/Source/C/Common/ParseInf.c               |  24 ++--
>  BaseTools/Source/C/Common/SimpleFileParsing.c      |  14 +--
>  BaseTools/Source/C/Common/TianoCompress.c          |   9 +-
>  BaseTools/Source/C/EfiRom/EfiRom.c                 | 120 ++++++++++++------
>  BaseTools/Source/C/GenBootSector/GenBootSector.c   |  43 ++++---
>  BaseTools/Source/C/GenCrc32/GenCrc32.c             |   3 +-
>  BaseTools/Source/C/GenFfs/GenFfs.c                 |  36 +++---
>  BaseTools/Source/C/GenFv/GenFv.c                   |   9 +-
>  BaseTools/Source/C/GenFv/GenFvInternalLib.c        |  83 ++++++++++--
>  BaseTools/Source/C/GenFw/Elf32Convert.c            |   8 ++
>  BaseTools/Source/C/GenFw/Elf64Convert.c            |  10 +-
>  BaseTools/Source/C/GenFw/ElfConvert.c              |   7 +-
>  BaseTools/Source/C/GenFw/GenFw.c                   |  20 ++-
>  BaseTools/Source/C/GenPage/GenPage.c               |  12 +-
>  BaseTools/Source/C/GenSec/GenSec.c                 |  27 +++-
>  BaseTools/Source/C/GenVtf/GenVtf.c                 | 117 ++++++++++++++++-
>  BaseTools/Source/C/LzmaCompress/LzmaCompress.c     |   6 +-
>  BaseTools/Source/C/Split/Split.c                   |  41 ++++--
>  BaseTools/Source/C/TianoCompress/TianoCompress.c   |  68 ++++++----
>  BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h    |   4 +-
>  .../Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h      |   6 +-
>  BaseTools/Source/C/VfrCompile/Pccts/h/DLexer.h     |   3 +
>  BaseTools/Source/C/VfrCompile/Pccts/h/DLexerBase.h |   4 +
>  BaseTools/Source/C/VfrCompile/VfrCompiler.cpp      | 140
> ++++++++++++++++++---
>  BaseTools/Source/C/VfrCompile/VfrCompiler.h        |   8 +-
>  BaseTools/Source/C/VfrCompile/VfrError.cpp         |   4 +-
>  BaseTools/Source/C/VfrCompile/VfrError.h           |  10 +-
>  BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp       |  29 +++--
>  BaseTools/Source/C/VfrCompile/VfrFormPkg.h         |  13 ++
>  BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp    |  54 +++++---
>  BaseTools/Source/C/VfrCompile/VfrUtilityLib.h      |  60 ++++++++-
>  BaseTools/Source/C/VolInfo/VolInfo.c               | 107 +++++++++++++---
>  40 files changed, 1004 insertions(+), 261 deletions(-)
> 
> --
> 1.9.5.msysgit.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to