Re: [edk2] [PATCH 00/52] Resolve issues for C source codes in BaseTools

2016-10-17 Thread Gao, Liming 
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 ; Gao, Liming ;
> Zhu, Yonghong ; Dong, Eric
> ; Bi, Dandan 
> 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 
> Cc: Yonghong Zhu 
> Cc: Eric Dong 
> Cc: Dandan Bi 
> 
> 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

[edk2] [PATCH 00/52] Resolve issues for C source codes in BaseTools

2016-10-12 Thread Hao Wu 
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 
Cc: Yonghong Zhu 
Cc: Eric Dong 
Cc: Dandan Bi 

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 ++