ASCII characters {|}~ should be printed by DumpHex. The problem is that
if you have a string like
{xizzy}~{foo|bar}~{quux}
in the dumped data, it will not appear as such in the *-delimited ASCII
column to the right, but as
.xizzy...foo.bar...quux.
which is less than ideal.
Most of the commit message was inspired by/shamelessly stolen from
Laszlo's example:
https://lists.01.org/pipermail/edk2-devel/2017-April/010266.html
Cc: Jaben Carsey <jaben.carsey@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Westfahl <jeff.westfahl@ni.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Commit bd3fc8133b ("ShellPkg/App: Fix memory leak and save resources.",
2016-05-20) added a FreePool() call for Split->SplitStdIn, near end of the
RunSplitCommand(), right after the same shell file was closed with
CloseFile(). The argument was:
> 1) RunSplitCommand() allocates the initial SplitStdOut via
> CreateFileInterfaceMem(). Free SplitStdIn after the swap to fix
> the memory leak.
There is no memory leak actually, and the FreePool() call in question
constitutes a double-free:
(a) This is how the handle is established:
ConvertEfiFileProtocolToShellHandle (
CreateFileInterfaceMem (Unicode),
NULL
);
CreateFileInterfaceMem() allocates an EFI_FILE_PROTOCOL_MEM object and
populates it fully. ConvertEfiFileProtocolToShellHandle() allocates
some administrative structures and links the EFI_FILE_PROTOCOL_MEM
object into "mFileHandleList".
(b) EFI_SHELL_PROTOCOL.CloseFile() is required to close the
SHELL_FILE_HANDLE and to release all associated data. Accordingly,
near the end of RunSplitCommand(), we have:
EfiShellClose()
ShellFileHandleRemove()
//
// undoes the effects of ConvertEfiFileProtocolToShellHandle()
//
ConvertShellHandleToEfiFileProtocol()
//
// note that this does not adjust the pointer value; it's a pure
// type cast
//
FileHandleClose()
FileInterfaceMemClose()
//
// tears down EFI_FILE_PROTOCOL_MEM completely, undoing the
// effects of CreateFileInterfaceMem ()
//
The FreePool() call added by bd3fc8133b conflicts with
SHELL_FREE_NON_NULL(This);
in FileInterfaceMemClose(), so remove it.
This error can be reproduced for example with:
> Shell> map | more
> 'more' is not recognized as an internal or external command, operable
> program, or script file.
which triggers:
> ASSERT MdeModulePkg/Core/Dxe/Mem/Pool.c(624): CR has Bad Signature
with the following stack dump:
> #0 0x000000007f6dc094 in CpuDeadLoop () at
> MdePkg/Library/BaseLib/CpuDeadLoop.c:37
> #1 0x000000007f6dd1b4 in DebugAssert (FileName=0x7f6ed9f0
> "MdeModulePkg/Core/Dxe/Mem/Pool.c", LineNumber=624,
> Description=0x7f6ed9d8 "CR has Bad Signature") at
> OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c:153
> #2 0x000000007f6d075d in CoreFreePoolI (Buffer=0x7e232c98,
> PoolType=0x7f6bc1c4) at MdeModulePkg/Core/Dxe/Mem/Pool.c:624
> #3 0x000000007f6d060e in CoreInternalFreePool (Buffer=0x7e232c98,
> PoolType=0x7f6bc1c4) at MdeModulePkg/Core/Dxe/Mem/Pool.c:529
> #4 0x000000007f6d0648 in CoreFreePool (Buffer=0x7e232c98) at
> MdeModulePkg/Core/Dxe/Mem/Pool.c:552
> #5 0x000000007d49fbf8 in FreePool (Buffer=0x7e232c98) at
> MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c:818
> #6 0x000000007d4875c3 in RunSplitCommand (CmdLine=0x7d898398,
> StdIn=0x0, StdOut=0x0) at ShellPkg/Application/Shell/Shell.c:1813
> #7 0x000000007d487d59 in ProcessNewSplitCommandLine
> (CmdLine=0x7d898398) at ShellPkg/Application/Shell/Shell.c:2121
> #8 0x000000007d488937 in RunShellCommand (CmdLine=0x7e233018,
> CommandStatus=0x0) at ShellPkg/Application/Shell/Shell.c:2670
> #9 0x000000007d488b0b in RunCommand (CmdLine=0x7e233018) at
> ShellPkg/Application/Shell/Shell.c:2732
> #10 0x000000007d4867c8 in DoShellPrompt () at
> ShellPkg/Application/Shell/Shell.c:1349
> #11 0x000000007d48524d in UefiMain (ImageHandle=0x7e24c898,
> SystemTable=0x7f5b6018) at ShellPkg/Application/Shell/Shell.c:631
Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Marvin Häuser <Marvin.Haeuser@outlook.com>
Cc: Qiu Shumin <shumin.qiu@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Fixes: bd3fc8133b
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
Reviewed-by: Marvin Häuser <Marvin.Haeuser@outlook.com>
The "SPLIT_LIST.SplitStdOut" and "SPLIT_LIST.SplitStdIn" members currently
have type (SHELL_FILE_HANDLE *). This is wrong; SHELL_FILE_HANDLE is
already a pointer, there's no need to store a pointer to a pointer.
The error is obvious if we check where and how these members are used:
- In the RunSplitCommand() function, these members are used (populated)
extensively; this function has to be updated in sync.
ConvertEfiFileProtocolToShellHandle() already returns the temporary
memory file created with CreateFileInterfaceMem() as SHELL_FILE_HANDLE,
not as (SHELL_FILE_HANDLE *).
- In particular, the ConvertShellHandleToEfiFileProtocol() calls need to
be dropped as well in RunSplitCommand(), since
EFI_SHELL_PROTOCOL.SetFilePosition() and EFI_SHELL_PROTOCOL.CloseFile()
take SHELL_FILE_HANDLE parameters, not (EFI_FILE_PROTOCOL *).
Given that ConvertShellHandleToEfiFileProtocol() only performs a
type-cast (it does not adjust any pointer values), *and*
SHELL_FILE_HANDLE -- taken by EFI_SHELL_PROTOCOL member functions -- is
actually a typedef to (VOID *) -- see more on this later --, this
conversion error hasn't been caught by compilers.
- In the ProcessNewSplitCommandLine() function, RunSplitCommand() is
called either initially (passing in NULL / NULL; no update needed), or
recursively (passing in Split->SplitStdIn / Split->SplitStdOut; again no
update is necessary beyond the RunSplitCommand() modification above).
- In the UpdateStdInStdOutStdErr() and RestoreStdInStdOutStdErr()
functions, said structure members are compared and assigned to
"EFI_SHELL_PARAMETERS_PROTOCOL.StdIn" and
"EFI_SHELL_PARAMETERS_PROTOCOL.StdOut", both of which have type
SHELL_FILE_HANDLE, *not* (SHELL_FILE_HANDLE *).
The compiler hasn't caught this error because of the fatally flawed type
definition of SHELL_FILE_HANDLE, namely
typedef VOID *SHELL_FILE_HANDLE;
Pointer-to-void silently converts to and from most other pointer types;
among them, pointer-to-pointer-to-void. That is also why no update is
necessary for UpdateStdInStdOutStdErr() and RestoreStdInStdOutStdErr()
in this fix.
(
Generally speaking, using (VOID *) typedefs for opaque handles is a tragic
mistake in all of the UEFI-related specifications; this practice defeats
any type checking that compilers might help programmers with. The right
way to define an opaque handle is as follows:
//
// Introduce the incomplete structure type, and the derived pointer
// type, in both the specification and the public edk2 headers. Note
// that the derived pointer type itself is a complete type, and it can
// be used freely by client code.
//
typedef struct SHELL_FILE *SHELL_FILE_HANDLE;
//
// Complete the structure type in the edk2 internal C source files.
//
struct SHELL_FILE {
//
// list fields
//
};
This way the structure size and members remain hidden from client code,
but the C compiler can nonetheless catch any invalid conversions between
incompatible XXX_HANDLE types.
)
Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Marvin Häuser <Marvin.Haeuser@outlook.com>
Cc: Qiu Shumin <shumin.qiu@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
The SMBIOS Type 0 BIOS segment field is currently displayed in decimal.
Since this field is likely to have a value like 0xE800 or 0xF000, using
hexadecimal seems like a better choice.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Westfahl <jeff.westfahl@ni.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
It is to align to the original behavior before "-ec" option was
added.
The patch also refines the code to make it more readable.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
Cc: Jim Dailey <Jim.Dailey@dell.com>
When user doesn't press key to exit the timeout waiting in Shell,
and there is no startup.nsh, Shell exits with failure status.
aaf51f08ee introduced this bug.
The patch fixes this issue.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Chen A Chen <chen.a.chen@intel.com>
Shell 2.2 spec defines =0x/=0X, =H/=h, =S, =L and =P for
hex number, hex array, ascii string, unicode string and
device path data.
The patch adds such support.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chen A Chen <chen.a.chen@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Jeff Fan <jeff.fan@intel.com>
According to Shell spec 2.2 '-exit' invocation option is used to specify
that after running the command line when launched, the UEFI Shell must
immediately exit.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chen A Chen <chen.a.chen@intel.com>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
In QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c,
there is a definition of mUefiShellFileGuid which is a constant reference
to the FILE_GUID as defined in ShellPkg/Application/Shell/Shell.inf.
To prevent the need for duplicating it to other modules, promote it to
a proper global GUID, and add it to the ShellPkg.dec package declaration.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
The format specifier for the LoadOptions field of the LoadedImage protocol
is "%s". However, the data in LoadOptions is often generic binary data. A
format specifier of "%x" is more appropriate for this field.
Using "dh -v" with format specifier "%s" on BIOS images based on EDK II
source before commit 891d844 can cause a crash.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Westfahl <jeff.westfahl@ni.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
The patch doesn't impact the functionality.
The rename also fixes the inconsistency between function
header comments and function parameters.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
There are cases that the operands of an expression are all with rank less
than UINT64/INT64 and the result of the expression is explicitly cast to
UINT64/INT64 to fit the target size.
An example will be:
UINT32 a,b;
// a and b can be any unsigned int type with rank less than UINT64, like
// UINT8, UINT16, etc.
UINT64 c;
c = (UINT64) (a + b);
Some static code checkers may warn that the expression result might
overflow within the rank of "int" (integer promotions) and the result is
then cast to a bigger size.
The commit refines codes by the following rules:
1). When the expression is possible to overflow the range of unsigned int/
int:
c = (UINT64)a + b;
2). When the expression will not overflow within the rank of "int", remove
the explicit type casts:
c = a + b;
3). When the expression will be cast to pointer of possible greater size:
UINT32 a,b;
VOID *c;
c = (VOID *)(UINTN)(a + b); --> c = (VOID *)((UINTN)a + b);
4). When one side of a comparison expression contains only operands with
rank less than UINT32:
UINT8 a;
UINT16 b;
UINTN c;
if ((UINTN)(a + b) > c) {...} --> if (((UINT32)a + b) > c) {...}
For rule 4), if we remove the 'UINTN' type cast like:
if (a + b > c) {...}
The VS compiler will complain with warning C4018 (signed/unsigned
mismatch, level 3 warning) due to promoting 'a + b' to type 'int'.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
For pointer subtraction, the result is of type "ptrdiff_t". According to
the C11 standard (Committee Draft - April 12, 2011):
"When two pointers are subtracted, both shall point to elements of the
same array object, or one past the last element of the array object; the
result is the difference of the subscripts of the two array elements. The
size of the result is implementation-defined, and its type (a signed
integer type) is ptrdiff_t defined in the <stddef.h> header. If the result
is not representable in an object of that type, the behavior is
undefined."
In our codes, there are cases that the pointer subtraction is not
performed by pointers to elements of the same array object. This might
lead to potential issues, since the behavior is undefined according to C11
standard.
Also, since the size of type "ptrdiff_t" is implementation-defined. Some
static code checkers may warn that the pointer subtraction might underflow
first and then being cast to a bigger size. For example:
UINT8 *Ptr1, *Ptr2;
UINTN PtrDiff;
...
PtrDiff = (UINTN) (Ptr1 - Ptr2);
The commit will refine the pointer subtraction expressions by casting each
pointer to UINTN first and then perform the subtraction:
PtrDiff = (UINTN) Ptr1 - (UINTN) Ptr2;
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
Current UefiDpLib implementation depends on TimerLib,
as different platforms may implement and use their
own TimerLib, it makes the dp command needs to be built
by platform. The TimerLib dependency can be removed by
using performance property configuration table to make
UefiDpLib to be generic.
Cc: Andrew Fish <afish@apple.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Cinnamon Shia <cinnamon.shia@hpe.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Andrew Fish <afish@apple.com>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Type 0: Update "EDD Enhanced Disk Driver)..." to
"EDD (Enhanced Disk Driver)..." for
STR_SMBIOSVIEW_PRINTINFO_EDD_ENHANCED_DRIVER
Type 3: Use L" Laptop" instead of L" LapTop" in
SystemEnclosureTypeTable to match SMBIOS spec.
Type 10: The BIT7 of Device Type is representing the
status of device whether it is enabled or disabled.
But current code is not considering the BIT7 and will
print "Undefined Value" for enabled device. Type 41
has same definition of Device Type, the code is
correct and will be applied to Type 10 by this patch.
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>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
Current PrintBitsInfo() will always print an additional trailing
" | " for the bit flags, for example,
Base Board Feature Flags: Hosting board | Replaceable |
Th patch is to eliminate trailing " | " in PrintBitsInfo(), then
the output will be like below
Base Board Feature Flags: Hosting board | Replaceable
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>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
Before the "cd fs0:dir" fix, CD only prints destination directory
when the destination contains ":".
However, the "cd fs0:dir" fix changed CD to always print destination
directory.
This patch changes CD to never print destination directory.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
Cc: Chris J Phillips <chrisp@hpe.com>
Reviewed-by: Tapan Shah <tapandshah@hpe.com>
The implementation was already there but through a private flag
"-_e". The patch removes "-_e" support and add "-ec" support.
Removing old "-_e" support makes the pci command more clean.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
https://bugzilla.tianocore.org/show_bug.cgi?id=354
The patch removes the local PCI definitions and uses the definitions
defined in MdePkg/Include/IndustryStandard folder.
There is no functionality impact.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Jaben Carsey <jarben.carsey@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=340
The decoding of TPM Device (Type 43) has been added at
e9f0be021b.
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>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
if we set a static IP using command
'ifconfig -s eth0 static 192.168.0.121 255.255.255.0 0.0.0.0'
The system says 'Failed to set address.' but using
'ifconfig -l', the static IP can be assigned successfully.
so we need to check the gateway validity before setting manual
address to keep the ifconfig -s command more consistent.
Signed-off-by: Zhang Lubo <lubo.zhang@intel.com>
Cc: Santhapur Naveen <naveens@amiindia.co.in>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>
Reviewed-by: Ye Ting <ting.ye@intel.com>
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=345
When I am adding SMBIOS spec 3.1.0 support, I found the decoding
of SMBIOS spec 3.0.0 for some definitions is missing.
Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=344
SlotType AGP8X was added in SMBIOS spec 2.3.4, but the decoding
of it is missing. I found it when I am adding SMBIOS spec 3.1.0
support.
Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
When we issue 'ifconfig6 -s <interface> auto' system hangs with
an ASSERT in StrLen. in IfConfig6SetInterfaceInfo, for 'auto' case
we added checks to rule out the invalid inputs like 'host', 'gw'
and 'dns'. To parse through this, we do a VarArg = VarArg->Next but
we dont check new VarArg before calling StrCmp. Fix with a check
in this patch.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hegde Nagaraj P <nagaraj-p.hegde@hpe.com>
Reviewed-by: Zhang Lubo <lubo.zhang@intel.com>
Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>
Reviewed-by: Sriram Subramanian <sriram-s@hpe.com>
Added decoding of the new SMBIOS Type 43 record.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Augustine Linson P <linson.augustine@hpe.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Jeff Westfahl
Ruiyu Ni
Laszlo Ersek
Dandan Bi
Chen A Chen
Ard Biesheuvel
Star Zeng
Hao Wu
Zhang Lubo
hegdenag
Linson Augustine