* [PATCH] ShellPkg/Application: Fix ">v" cannot update environment variable
@ 2016-12-08 1:45 Ruiyu Ni
2016-12-08 17:03 ` Shah, Tapan
2016-12-08 21:59 ` Carsey, Jaben
0 siblings, 2 replies; 3+ messages in thread
From: Ruiyu Ni @ 2016-12-08 1:45 UTC (permalink / raw)
To: edk2-devel; +Cc: Chen A Chen, Jaben Carsey
From: Chen A Chen <chen.a.chen@intel.com>
When ">v" is used to redirect the command output to environment
variable (e.g.: "echo xxx >v yyy"), we only called SetVariable()
to update the variable storage but forgot to update the cached
environment variables in gShellEnvVarList.
When updating the variable storage, the existing code unnecessary
saved the ending NULL character into variable storage.
The patch fixes all the above issues.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chen A Chen <chen.a.chen@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
---
ShellPkg/Application/Shell/FileHandleWrappers.c | 170 ++++++++++++++++++------
ShellPkg/Application/Shell/ShellEnvVar.c | 12 +-
ShellPkg/Application/Shell/ShellProtocol.c | 4 +-
3 files changed, 142 insertions(+), 44 deletions(-)
diff --git a/ShellPkg/Application/Shell/FileHandleWrappers.c b/ShellPkg/Application/Shell/FileHandleWrappers.c
index 18f4169..3c11d82 100644
--- a/ShellPkg/Application/Shell/FileHandleWrappers.c
+++ b/ShellPkg/Application/Shell/FileHandleWrappers.c
@@ -1040,6 +1040,7 @@ FileInterfaceEnvClose(
UINTN NewSize;
EFI_STATUS Status;
BOOLEAN Volatile;
+ UINTN TotalSize;
//
// Most if not all UEFI commands will have an '\r\n' at the end of any output.
@@ -1049,6 +1050,7 @@ FileInterfaceEnvClose(
//
NewBuffer = NULL;
NewSize = 0;
+ TotalSize = 0;
Status = IsVolatileEnv (((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, &Volatile);
if (EFI_ERROR (Status)) {
@@ -1057,7 +1059,8 @@ FileInterfaceEnvClose(
Status = SHELL_GET_ENVIRONMENT_VARIABLE(((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, &NewSize, NewBuffer);
if (Status == EFI_BUFFER_TOO_SMALL) {
- NewBuffer = AllocateZeroPool(NewSize + sizeof(CHAR16));
+ TotalSize = NewSize + sizeof (CHAR16);
+ NewBuffer = AllocateZeroPool (TotalSize);
if (NewBuffer == NULL) {
return EFI_OUT_OF_RESOURCES;
}
@@ -1066,17 +1069,43 @@ FileInterfaceEnvClose(
if (!EFI_ERROR(Status) && NewBuffer != NULL) {
- if (StrSize(NewBuffer) > 6)
- {
- if ((((CHAR16*)NewBuffer)[(StrSize(NewBuffer)/2) - 2] == CHAR_LINEFEED)
- && (((CHAR16*)NewBuffer)[(StrSize(NewBuffer)/2) - 3] == CHAR_CARRIAGE_RETURN)) {
- ((CHAR16*)NewBuffer)[(StrSize(NewBuffer)/2) - 3] = CHAR_NULL;
+ if (TotalSize / sizeof (CHAR16) >= 3) {
+ if ( (((CHAR16*)NewBuffer)[TotalSize / sizeof (CHAR16) - 2] == CHAR_LINEFEED) &&
+ (((CHAR16*)NewBuffer)[TotalSize / sizeof (CHAR16) - 3] == CHAR_CARRIAGE_RETURN)
+ ) {
+ ((CHAR16*)NewBuffer)[TotalSize / sizeof (CHAR16) - 3] = CHAR_NULL;
}
if (Volatile) {
- Status = SHELL_SET_ENVIRONMENT_VARIABLE_V(((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, StrSize(NewBuffer), NewBuffer);
+ Status = SHELL_SET_ENVIRONMENT_VARIABLE_V (
+ ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
+ TotalSize - sizeof (CHAR16),
+ NewBuffer
+ );
+
+ if (!EFI_ERROR(Status)) {
+ Status = ShellAddEnvVarToList (
+ ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
+ NewBuffer,
+ TotalSize,
+ EFI_VARIABLE_BOOTSERVICE_ACCESS
+ );
+ }
} else {
- Status = SHELL_SET_ENVIRONMENT_VARIABLE_NV(((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, StrSize(NewBuffer), NewBuffer);
+ Status = SHELL_SET_ENVIRONMENT_VARIABLE_NV (
+ ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
+ TotalSize - sizeof (CHAR16),
+ NewBuffer
+ );
+
+ if (!EFI_ERROR(Status)) {
+ Status = ShellAddEnvVarToList (
+ ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
+ NewBuffer,
+ TotalSize,
+ EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
+ );
+ }
}
}
}
@@ -1128,12 +1157,14 @@ FileInterfaceEnvRead(
/**
File style interface for Volatile Environment Variable (Write).
+ This function also caches the environment variable into gShellEnvVarList.
@param[in] This The pointer to the EFI_FILE_PROTOCOL object.
@param[in, out] BufferSize Size in bytes of Buffer.
@param[in] Buffer The pointer to the buffer to write.
- @retval EFI_SUCCESS The data was read.
+ @retval EFI_SUCCESS The data was successfully write to variable.
+ @retval SHELL_OUT_OF_RESOURCES A memory allocation failed.
**/
EFI_STATUS
EFIAPI
@@ -1146,41 +1177,71 @@ FileInterfaceEnvVolWrite(
VOID* NewBuffer;
UINTN NewSize;
EFI_STATUS Status;
+ UINTN TotalSize;
NewBuffer = NULL;
NewSize = 0;
+ TotalSize = 0;
Status = SHELL_GET_ENVIRONMENT_VARIABLE(((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, &NewSize, NewBuffer);
- if (Status == EFI_BUFFER_TOO_SMALL){
- NewBuffer = AllocateZeroPool(NewSize + *BufferSize + sizeof(CHAR16));
+ if (Status == EFI_BUFFER_TOO_SMALL) {
+ TotalSize = NewSize + *BufferSize + sizeof (CHAR16);
+ } else if (Status == EFI_NOT_FOUND) {
+ TotalSize = *BufferSize + sizeof(CHAR16);
+ } else {
+ return Status;
+ }
+
+ NewBuffer = AllocateZeroPool (TotalSize);
+ if (NewBuffer == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ if (Status == EFI_BUFFER_TOO_SMALL) {
Status = SHELL_GET_ENVIRONMENT_VARIABLE(((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, &NewSize, NewBuffer);
}
- if (!EFI_ERROR(Status) && NewBuffer != NULL) {
- while (((CHAR16*)NewBuffer)[NewSize/2] == CHAR_NULL) {
- //
- // We want to overwrite the CHAR_NULL
- //
- NewSize -= 2;
- }
- CopyMem((UINT8*)NewBuffer + NewSize + 2, Buffer, *BufferSize);
- Status = SHELL_SET_ENVIRONMENT_VARIABLE_V(((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, StrSize(NewBuffer), NewBuffer);
- FreePool(NewBuffer);
- return (Status);
- } else {
- SHELL_FREE_NON_NULL(NewBuffer);
- return (SHELL_SET_ENVIRONMENT_VARIABLE_V(((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, *BufferSize, Buffer));
+
+ if (EFI_ERROR (Status) && Status != EFI_NOT_FOUND) {
+ FreePool (NewBuffer);
+ return Status;
+ }
+
+ CopyMem ((UINT8*)NewBuffer + NewSize, Buffer, *BufferSize);
+ Status = ShellAddEnvVarToList (
+ ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
+ NewBuffer,
+ TotalSize,
+ EFI_VARIABLE_BOOTSERVICE_ACCESS
+ );
+ if (EFI_ERROR(Status)) {
+ FreePool (NewBuffer);
+ return Status;
+ }
+
+ Status = SHELL_SET_ENVIRONMENT_VARIABLE_V (
+ ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
+ TotalSize - sizeof (CHAR16),
+ NewBuffer
+ );
+ if (EFI_ERROR(Status)) {
+ ShellRemvoeEnvVarFromList (((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name);
}
+
+ FreePool (NewBuffer);
+ return Status;
}
/**
File style interface for Non Volatile Environment Variable (Write).
+ This function also caches the environment variable into gShellEnvVarList.
@param[in] This The pointer to the EFI_FILE_PROTOCOL object.
@param[in, out] BufferSize Size in bytes of Buffer.
@param[in] Buffer The pointer to the buffer to write.
- @retval EFI_SUCCESS The data was read.
+ @retval EFI_SUCCESS The data was successfully write to variable.
+ @retval SHELL_OUT_OF_RESOURCES A memory allocation failed.
**/
EFI_STATUS
EFIAPI
@@ -1193,27 +1254,58 @@ FileInterfaceEnvNonVolWrite(
VOID* NewBuffer;
UINTN NewSize;
EFI_STATUS Status;
+ UINTN TotalSize;
NewBuffer = NULL;
NewSize = 0;
+ TotalSize = 0;
Status = SHELL_GET_ENVIRONMENT_VARIABLE(((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, &NewSize, NewBuffer);
- if (Status == EFI_BUFFER_TOO_SMALL){
- NewBuffer = AllocateZeroPool(NewSize + *BufferSize);
+ if (Status == EFI_BUFFER_TOO_SMALL) {
+ TotalSize = NewSize + *BufferSize + sizeof (CHAR16);
+ } else if (Status == EFI_NOT_FOUND) {
+ TotalSize = *BufferSize + sizeof (CHAR16);
+ } else {
+ return Status;
+ }
+
+ NewBuffer = AllocateZeroPool (TotalSize);
+ if (NewBuffer == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ if (Status == EFI_BUFFER_TOO_SMALL) {
Status = SHELL_GET_ENVIRONMENT_VARIABLE(((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, &NewSize, NewBuffer);
}
- if (!EFI_ERROR(Status)) {
- CopyMem((UINT8*)NewBuffer + NewSize, Buffer, *BufferSize);
- return (SHELL_SET_ENVIRONMENT_VARIABLE_NV(
- ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
- NewSize + *BufferSize,
- NewBuffer));
- } else {
- return (SHELL_SET_ENVIRONMENT_VARIABLE_NV(
- ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
- *BufferSize,
- Buffer));
+
+ if (EFI_ERROR(Status) && Status != EFI_NOT_FOUND) {
+ FreePool (NewBuffer);
+ return Status;
+ }
+
+ CopyMem ((UINT8*) NewBuffer + NewSize, Buffer, *BufferSize);
+ Status = ShellAddEnvVarToList (
+ ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
+ NewBuffer,
+ TotalSize,
+ EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
+ );
+ if (EFI_ERROR (Status)) {
+ FreePool (NewBuffer);
+ return Status;
}
+
+ Status = SHELL_SET_ENVIRONMENT_VARIABLE_NV (
+ ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
+ TotalSize - sizeof (CHAR16),
+ NewBuffer
+ );
+ if (EFI_ERROR (Status)) {
+ ShellRemvoeEnvVarFromList (((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name);
+ }
+
+ FreePool (NewBuffer);
+ return Status;
}
/**
diff --git a/ShellPkg/Application/Shell/ShellEnvVar.c b/ShellPkg/Application/Shell/ShellEnvVar.c
index 6d8c79b..4c49c1c 100644
--- a/ShellPkg/Application/Shell/ShellEnvVar.c
+++ b/ShellPkg/Application/Shell/ShellEnvVar.c
@@ -178,7 +178,10 @@ GetEnvironmentVariableList(
Status = EFI_OUT_OF_RESOURCES;
} else {
ValSize = ValBufferSize;
- VarList->Val = AllocateZeroPool(ValSize);
+ //
+ // We need another CHAR16 to save '\0' in VarList->Val.
+ //
+ VarList->Val = AllocateZeroPool (ValSize + sizeof (CHAR16));
if (VarList->Val == NULL) {
SHELL_FREE_NON_NULL(VarList);
Status = EFI_OUT_OF_RESOURCES;
@@ -188,7 +191,10 @@ GetEnvironmentVariableList(
if (Status == EFI_BUFFER_TOO_SMALL){
ValBufferSize = ValSize > ValBufferSize * 2 ? ValSize : ValBufferSize * 2;
SHELL_FREE_NON_NULL (VarList->Val);
- VarList->Val = AllocateZeroPool(ValBufferSize);
+ //
+ // We need another CHAR16 to save '\0' in VarList->Val.
+ //
+ VarList->Val = AllocateZeroPool (ValBufferSize + sizeof (CHAR16));
if (VarList->Val == NULL) {
SHELL_FREE_NON_NULL(VarList);
Status = EFI_OUT_OF_RESOURCES;
@@ -272,7 +278,7 @@ SetEnvironmentVariableList(
; !IsNull(ListHead, &Node->Link)
; Node = (ENV_VAR_LIST*)GetNextNode(ListHead, &Node->Link)
){
- Size = StrSize(Node->Val);
+ Size = StrSize (Node->Val) - sizeof (CHAR16);
if (Node->Atts & EFI_VARIABLE_NON_VOLATILE) {
Status = SHELL_SET_ENVIRONMENT_VARIABLE_NV(Node->Key, Size, Node->Val);
} else {
diff --git a/ShellPkg/Application/Shell/ShellProtocol.c b/ShellPkg/Application/Shell/ShellProtocol.c
index 04b66c5..12c7c40 100644
--- a/ShellPkg/Application/Shell/ShellProtocol.c
+++ b/ShellPkg/Application/Shell/ShellProtocol.c
@@ -2872,8 +2872,8 @@ InternalEfiShellSetEnv(
);
if (!EFI_ERROR (Status)) {
Status = Volatile
- ? SHELL_SET_ENVIRONMENT_VARIABLE_V(Name, StrSize(Value), Value)
- : SHELL_SET_ENVIRONMENT_VARIABLE_NV(Name, StrSize(Value), Value);
+ ? SHELL_SET_ENVIRONMENT_VARIABLE_V (Name, StrSize (Value) - sizeof (CHAR16), Value)
+ : SHELL_SET_ENVIRONMENT_VARIABLE_NV (Name, StrSize (Value) - sizeof (CHAR16), Value);
if (EFI_ERROR (Status)) {
ShellRemvoeEnvVarFromList(Name);
}
--
2.9.0.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] ShellPkg/Application: Fix ">v" cannot update environment variable
2016-12-08 1:45 [PATCH] ShellPkg/Application: Fix ">v" cannot update environment variable Ruiyu Ni
@ 2016-12-08 17:03 ` Shah, Tapan
2016-12-08 21:59 ` Carsey, Jaben
1 sibling, 0 replies; 3+ messages in thread
From: Shah, Tapan @ 2016-12-08 17:03 UTC (permalink / raw)
To: Ruiyu Ni, edk2-devel@lists.01.org; +Cc: Jaben Carsey, Chen A Chen
Reviewed-by: Tapan Shah <tapandshah@hpe.com>
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Wednesday, December 07, 2016 7:45 PM
To: edk2-devel@lists.01.org
Cc: Jaben Carsey <jaben.carsey@intel.com>; Chen A Chen <chen.a.chen@intel.com>
Subject: [edk2] [PATCH] ShellPkg/Application: Fix ">v" cannot update environment variable
From: Chen A Chen <chen.a.chen@intel.com>
When ">v" is used to redirect the command output to environment
variable (e.g.: "echo xxx >v yyy"), we only called SetVariable()
to update the variable storage but forgot to update the cached
environment variables in gShellEnvVarList.
When updating the variable storage, the existing code unnecessary
saved the ending NULL character into variable storage.
The patch fixes all the above issues.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chen A Chen <chen.a.chen@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
---
ShellPkg/Application/Shell/FileHandleWrappers.c | 170 ++++++++++++++++++------
ShellPkg/Application/Shell/ShellEnvVar.c | 12 +-
ShellPkg/Application/Shell/ShellProtocol.c | 4 +-
3 files changed, 142 insertions(+), 44 deletions(-)
diff --git a/ShellPkg/Application/Shell/FileHandleWrappers.c b/ShellPkg/Application/Shell/FileHandleWrappers.c
index 18f4169..3c11d82 100644
--- a/ShellPkg/Application/Shell/FileHandleWrappers.c
+++ b/ShellPkg/Application/Shell/FileHandleWrappers.c
@@ -1040,6 +1040,7 @@ FileInterfaceEnvClose(
UINTN NewSize;
EFI_STATUS Status;
BOOLEAN Volatile;
+ UINTN TotalSize;
//
// Most if not all UEFI commands will have an '\r\n' at the end of any output.
@@ -1049,6 +1050,7 @@ FileInterfaceEnvClose(
//
NewBuffer = NULL;
NewSize = 0;
+ TotalSize = 0;
Status = IsVolatileEnv (((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, &Volatile);
if (EFI_ERROR (Status)) {
@@ -1057,7 +1059,8 @@ FileInterfaceEnvClose(
Status = SHELL_GET_ENVIRONMENT_VARIABLE(((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, &NewSize, NewBuffer);
if (Status == EFI_BUFFER_TOO_SMALL) {
- NewBuffer = AllocateZeroPool(NewSize + sizeof(CHAR16));
+ TotalSize = NewSize + sizeof (CHAR16);
+ NewBuffer = AllocateZeroPool (TotalSize);
if (NewBuffer == NULL) {
return EFI_OUT_OF_RESOURCES;
}
@@ -1066,17 +1069,43 @@ FileInterfaceEnvClose(
if (!EFI_ERROR(Status) && NewBuffer != NULL) {
- if (StrSize(NewBuffer) > 6)
- {
- if ((((CHAR16*)NewBuffer)[(StrSize(NewBuffer)/2) - 2] == CHAR_LINEFEED)
- && (((CHAR16*)NewBuffer)[(StrSize(NewBuffer)/2) - 3] == CHAR_CARRIAGE_RETURN)) {
- ((CHAR16*)NewBuffer)[(StrSize(NewBuffer)/2) - 3] = CHAR_NULL;
+ if (TotalSize / sizeof (CHAR16) >= 3) {
+ if ( (((CHAR16*)NewBuffer)[TotalSize / sizeof (CHAR16) - 2] == CHAR_LINEFEED) &&
+ (((CHAR16*)NewBuffer)[TotalSize / sizeof (CHAR16) - 3] == CHAR_CARRIAGE_RETURN)
+ ) {
+ ((CHAR16*)NewBuffer)[TotalSize / sizeof (CHAR16) - 3] = CHAR_NULL;
}
if (Volatile) {
- Status = SHELL_SET_ENVIRONMENT_VARIABLE_V(((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, StrSize(NewBuffer), NewBuffer);
+ Status = SHELL_SET_ENVIRONMENT_VARIABLE_V (
+ ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
+ TotalSize - sizeof (CHAR16),
+ NewBuffer
+ );
+
+ if (!EFI_ERROR(Status)) {
+ Status = ShellAddEnvVarToList (
+ ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
+ NewBuffer,
+ TotalSize,
+ EFI_VARIABLE_BOOTSERVICE_ACCESS
+ );
+ }
} else {
- Status = SHELL_SET_ENVIRONMENT_VARIABLE_NV(((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, StrSize(NewBuffer), NewBuffer);
+ Status = SHELL_SET_ENVIRONMENT_VARIABLE_NV (
+ ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
+ TotalSize - sizeof (CHAR16),
+ NewBuffer
+ );
+
+ if (!EFI_ERROR(Status)) {
+ Status = ShellAddEnvVarToList (
+ ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
+ NewBuffer,
+ TotalSize,
+ EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
+ );
+ }
}
}
}
@@ -1128,12 +1157,14 @@ FileInterfaceEnvRead(
/**
File style interface for Volatile Environment Variable (Write).
+ This function also caches the environment variable into gShellEnvVarList.
@param[in] This The pointer to the EFI_FILE_PROTOCOL object.
@param[in, out] BufferSize Size in bytes of Buffer.
@param[in] Buffer The pointer to the buffer to write.
- @retval EFI_SUCCESS The data was read.
+ @retval EFI_SUCCESS The data was successfully write to variable.
+ @retval SHELL_OUT_OF_RESOURCES A memory allocation failed.
**/
EFI_STATUS
EFIAPI
@@ -1146,41 +1177,71 @@ FileInterfaceEnvVolWrite(
VOID* NewBuffer;
UINTN NewSize;
EFI_STATUS Status;
+ UINTN TotalSize;
NewBuffer = NULL;
NewSize = 0;
+ TotalSize = 0;
Status = SHELL_GET_ENVIRONMENT_VARIABLE(((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, &NewSize, NewBuffer);
- if (Status == EFI_BUFFER_TOO_SMALL){
- NewBuffer = AllocateZeroPool(NewSize + *BufferSize + sizeof(CHAR16));
+ if (Status == EFI_BUFFER_TOO_SMALL) {
+ TotalSize = NewSize + *BufferSize + sizeof (CHAR16);
+ } else if (Status == EFI_NOT_FOUND) {
+ TotalSize = *BufferSize + sizeof(CHAR16);
+ } else {
+ return Status;
+ }
+
+ NewBuffer = AllocateZeroPool (TotalSize);
+ if (NewBuffer == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ if (Status == EFI_BUFFER_TOO_SMALL) {
Status = SHELL_GET_ENVIRONMENT_VARIABLE(((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, &NewSize, NewBuffer);
}
- if (!EFI_ERROR(Status) && NewBuffer != NULL) {
- while (((CHAR16*)NewBuffer)[NewSize/2] == CHAR_NULL) {
- //
- // We want to overwrite the CHAR_NULL
- //
- NewSize -= 2;
- }
- CopyMem((UINT8*)NewBuffer + NewSize + 2, Buffer, *BufferSize);
- Status = SHELL_SET_ENVIRONMENT_VARIABLE_V(((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, StrSize(NewBuffer), NewBuffer);
- FreePool(NewBuffer);
- return (Status);
- } else {
- SHELL_FREE_NON_NULL(NewBuffer);
- return (SHELL_SET_ENVIRONMENT_VARIABLE_V(((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, *BufferSize, Buffer));
+
+ if (EFI_ERROR (Status) && Status != EFI_NOT_FOUND) {
+ FreePool (NewBuffer);
+ return Status;
+ }
+
+ CopyMem ((UINT8*)NewBuffer + NewSize, Buffer, *BufferSize);
+ Status = ShellAddEnvVarToList (
+ ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
+ NewBuffer,
+ TotalSize,
+ EFI_VARIABLE_BOOTSERVICE_ACCESS
+ );
+ if (EFI_ERROR(Status)) {
+ FreePool (NewBuffer);
+ return Status;
+ }
+
+ Status = SHELL_SET_ENVIRONMENT_VARIABLE_V (
+ ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
+ TotalSize - sizeof (CHAR16),
+ NewBuffer
+ );
+ if (EFI_ERROR(Status)) {
+ ShellRemvoeEnvVarFromList (((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name);
}
+
+ FreePool (NewBuffer);
+ return Status;
}
/**
File style interface for Non Volatile Environment Variable (Write).
+ This function also caches the environment variable into gShellEnvVarList.
@param[in] This The pointer to the EFI_FILE_PROTOCOL object.
@param[in, out] BufferSize Size in bytes of Buffer.
@param[in] Buffer The pointer to the buffer to write.
- @retval EFI_SUCCESS The data was read.
+ @retval EFI_SUCCESS The data was successfully write to variable.
+ @retval SHELL_OUT_OF_RESOURCES A memory allocation failed.
**/
EFI_STATUS
EFIAPI
@@ -1193,27 +1254,58 @@ FileInterfaceEnvNonVolWrite(
VOID* NewBuffer;
UINTN NewSize;
EFI_STATUS Status;
+ UINTN TotalSize;
NewBuffer = NULL;
NewSize = 0;
+ TotalSize = 0;
Status = SHELL_GET_ENVIRONMENT_VARIABLE(((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, &NewSize, NewBuffer);
- if (Status == EFI_BUFFER_TOO_SMALL){
- NewBuffer = AllocateZeroPool(NewSize + *BufferSize);
+ if (Status == EFI_BUFFER_TOO_SMALL) {
+ TotalSize = NewSize + *BufferSize + sizeof (CHAR16);
+ } else if (Status == EFI_NOT_FOUND) {
+ TotalSize = *BufferSize + sizeof (CHAR16);
+ } else {
+ return Status;
+ }
+
+ NewBuffer = AllocateZeroPool (TotalSize);
+ if (NewBuffer == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ if (Status == EFI_BUFFER_TOO_SMALL) {
Status = SHELL_GET_ENVIRONMENT_VARIABLE(((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, &NewSize, NewBuffer);
}
- if (!EFI_ERROR(Status)) {
- CopyMem((UINT8*)NewBuffer + NewSize, Buffer, *BufferSize);
- return (SHELL_SET_ENVIRONMENT_VARIABLE_NV(
- ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
- NewSize + *BufferSize,
- NewBuffer));
- } else {
- return (SHELL_SET_ENVIRONMENT_VARIABLE_NV(
- ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
- *BufferSize,
- Buffer));
+
+ if (EFI_ERROR(Status) && Status != EFI_NOT_FOUND) {
+ FreePool (NewBuffer);
+ return Status;
+ }
+
+ CopyMem ((UINT8*) NewBuffer + NewSize, Buffer, *BufferSize);
+ Status = ShellAddEnvVarToList (
+ ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
+ NewBuffer,
+ TotalSize,
+ EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
+ );
+ if (EFI_ERROR (Status)) {
+ FreePool (NewBuffer);
+ return Status;
}
+
+ Status = SHELL_SET_ENVIRONMENT_VARIABLE_NV (
+ ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
+ TotalSize - sizeof (CHAR16),
+ NewBuffer
+ );
+ if (EFI_ERROR (Status)) {
+ ShellRemvoeEnvVarFromList (((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name);
+ }
+
+ FreePool (NewBuffer);
+ return Status;
}
/**
diff --git a/ShellPkg/Application/Shell/ShellEnvVar.c b/ShellPkg/Application/Shell/ShellEnvVar.c
index 6d8c79b..4c49c1c 100644
--- a/ShellPkg/Application/Shell/ShellEnvVar.c
+++ b/ShellPkg/Application/Shell/ShellEnvVar.c
@@ -178,7 +178,10 @@ GetEnvironmentVariableList(
Status = EFI_OUT_OF_RESOURCES;
} else {
ValSize = ValBufferSize;
- VarList->Val = AllocateZeroPool(ValSize);
+ //
+ // We need another CHAR16 to save '\0' in VarList->Val.
+ //
+ VarList->Val = AllocateZeroPool (ValSize + sizeof (CHAR16));
if (VarList->Val == NULL) {
SHELL_FREE_NON_NULL(VarList);
Status = EFI_OUT_OF_RESOURCES;
@@ -188,7 +191,10 @@ GetEnvironmentVariableList(
if (Status == EFI_BUFFER_TOO_SMALL){
ValBufferSize = ValSize > ValBufferSize * 2 ? ValSize : ValBufferSize * 2;
SHELL_FREE_NON_NULL (VarList->Val);
- VarList->Val = AllocateZeroPool(ValBufferSize);
+ //
+ // We need another CHAR16 to save '\0' in VarList->Val.
+ //
+ VarList->Val = AllocateZeroPool (ValBufferSize + sizeof (CHAR16));
if (VarList->Val == NULL) {
SHELL_FREE_NON_NULL(VarList);
Status = EFI_OUT_OF_RESOURCES;
@@ -272,7 +278,7 @@ SetEnvironmentVariableList(
; !IsNull(ListHead, &Node->Link)
; Node = (ENV_VAR_LIST*)GetNextNode(ListHead, &Node->Link)
){
- Size = StrSize(Node->Val);
+ Size = StrSize (Node->Val) - sizeof (CHAR16);
if (Node->Atts & EFI_VARIABLE_NON_VOLATILE) {
Status = SHELL_SET_ENVIRONMENT_VARIABLE_NV(Node->Key, Size, Node->Val);
} else {
diff --git a/ShellPkg/Application/Shell/ShellProtocol.c b/ShellPkg/Application/Shell/ShellProtocol.c
index 04b66c5..12c7c40 100644
--- a/ShellPkg/Application/Shell/ShellProtocol.c
+++ b/ShellPkg/Application/Shell/ShellProtocol.c
@@ -2872,8 +2872,8 @@ InternalEfiShellSetEnv(
);
if (!EFI_ERROR (Status)) {
Status = Volatile
- ? SHELL_SET_ENVIRONMENT_VARIABLE_V(Name, StrSize(Value), Value)
- : SHELL_SET_ENVIRONMENT_VARIABLE_NV(Name, StrSize(Value), Value);
+ ? SHELL_SET_ENVIRONMENT_VARIABLE_V (Name, StrSize (Value) - sizeof (CHAR16), Value)
+ : SHELL_SET_ENVIRONMENT_VARIABLE_NV (Name, StrSize (Value) - sizeof (CHAR16), Value);
if (EFI_ERROR (Status)) {
ShellRemvoeEnvVarFromList(Name);
}
--
2.9.0.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] ShellPkg/Application: Fix ">v" cannot update environment variable
2016-12-08 1:45 [PATCH] ShellPkg/Application: Fix ">v" cannot update environment variable Ruiyu Ni
2016-12-08 17:03 ` Shah, Tapan
@ 2016-12-08 21:59 ` Carsey, Jaben
1 sibling, 0 replies; 3+ messages in thread
From: Carsey, Jaben @ 2016-12-08 21:59 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Chen, Chen A, Carsey, Jaben
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Wednesday, December 7, 2016 5:45 PM
> To: edk2-devel@lists.01.org
> Cc: Chen, Chen A <chen.a.chen@intel.com>; Carsey, Jaben
> <jaben.carsey@intel.com>
> Subject: [PATCH] ShellPkg/Application: Fix ">v" cannot update environment
> variable
> Importance: High
>
> From: Chen A Chen <chen.a.chen@intel.com>
>
> When ">v" is used to redirect the command output to environment variable
> (e.g.: "echo xxx >v yyy"), we only called SetVariable() to update the variable
> storage but forgot to update the cached environment variables in
> gShellEnvVarList.
> When updating the variable storage, the existing code unnecessary saved the
> ending NULL character into variable storage.
>
> The patch fixes all the above issues.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chen A Chen <chen.a.chen@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> ---
> ShellPkg/Application/Shell/FileHandleWrappers.c | 170 ++++++++++++++++++---
> ---
> ShellPkg/Application/Shell/ShellEnvVar.c | 12 +-
> ShellPkg/Application/Shell/ShellProtocol.c | 4 +-
> 3 files changed, 142 insertions(+), 44 deletions(-)
>
> diff --git a/ShellPkg/Application/Shell/FileHandleWrappers.c
> b/ShellPkg/Application/Shell/FileHandleWrappers.c
> index 18f4169..3c11d82 100644
> --- a/ShellPkg/Application/Shell/FileHandleWrappers.c
> +++ b/ShellPkg/Application/Shell/FileHandleWrappers.c
> @@ -1040,6 +1040,7 @@ FileInterfaceEnvClose(
> UINTN NewSize;
> EFI_STATUS Status;
> BOOLEAN Volatile;
> + UINTN TotalSize;
>
> //
> // Most if not all UEFI commands will have an '\r\n' at the end of any output.
> @@ -1049,6 +1050,7 @@ FileInterfaceEnvClose(
> //
> NewBuffer = NULL;
> NewSize = 0;
> + TotalSize = 0;
>
> Status = IsVolatileEnv (((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
> &Volatile);
> if (EFI_ERROR (Status)) {
> @@ -1057,7 +1059,8 @@ FileInterfaceEnvClose(
>
> Status =
> SHELL_GET_ENVIRONMENT_VARIABLE(((EFI_FILE_PROTOCOL_ENVIRONMENT*
> )This)->Name, &NewSize, NewBuffer);
> if (Status == EFI_BUFFER_TOO_SMALL) {
> - NewBuffer = AllocateZeroPool(NewSize + sizeof(CHAR16));
> + TotalSize = NewSize + sizeof (CHAR16);
> + NewBuffer = AllocateZeroPool (TotalSize);
> if (NewBuffer == NULL) {
> return EFI_OUT_OF_RESOURCES;
> }
> @@ -1066,17 +1069,43 @@ FileInterfaceEnvClose(
>
> if (!EFI_ERROR(Status) && NewBuffer != NULL) {
>
> - if (StrSize(NewBuffer) > 6)
> - {
> - if ((((CHAR16*)NewBuffer)[(StrSize(NewBuffer)/2) - 2] == CHAR_LINEFEED)
> - && (((CHAR16*)NewBuffer)[(StrSize(NewBuffer)/2) - 3] ==
> CHAR_CARRIAGE_RETURN)) {
> - ((CHAR16*)NewBuffer)[(StrSize(NewBuffer)/2) - 3] = CHAR_NULL;
> + if (TotalSize / sizeof (CHAR16) >= 3) {
> + if ( (((CHAR16*)NewBuffer)[TotalSize / sizeof (CHAR16) - 2] ==
> CHAR_LINEFEED) &&
> + (((CHAR16*)NewBuffer)[TotalSize / sizeof (CHAR16) - 3] ==
> CHAR_CARRIAGE_RETURN)
> + ) {
> + ((CHAR16*)NewBuffer)[TotalSize / sizeof (CHAR16) - 3] =
> + CHAR_NULL;
> }
>
> if (Volatile) {
> - Status =
> SHELL_SET_ENVIRONMENT_VARIABLE_V(((EFI_FILE_PROTOCOL_ENVIRONMEN
> T*)This)->Name, StrSize(NewBuffer), NewBuffer);
> + Status = SHELL_SET_ENVIRONMENT_VARIABLE_V (
> + ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
> + TotalSize - sizeof (CHAR16),
> + NewBuffer
> + );
> +
> + if (!EFI_ERROR(Status)) {
> + Status = ShellAddEnvVarToList (
> + ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
> + NewBuffer,
> + TotalSize,
> + EFI_VARIABLE_BOOTSERVICE_ACCESS
> + );
> + }
> } else {
> - Status =
> SHELL_SET_ENVIRONMENT_VARIABLE_NV(((EFI_FILE_PROTOCOL_ENVIRONME
> NT*)This)->Name, StrSize(NewBuffer), NewBuffer);
> + Status = SHELL_SET_ENVIRONMENT_VARIABLE_NV (
> + ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
> + TotalSize - sizeof (CHAR16),
> + NewBuffer
> + );
> +
> + if (!EFI_ERROR(Status)) {
> + Status = ShellAddEnvVarToList (
> + ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
> + NewBuffer,
> + TotalSize,
> + EFI_VARIABLE_NON_VOLATILE |
> EFI_VARIABLE_BOOTSERVICE_ACCESS
> + );
> + }
> }
> }
> }
> @@ -1128,12 +1157,14 @@ FileInterfaceEnvRead(
>
> /**
> File style interface for Volatile Environment Variable (Write).
> + This function also caches the environment variable into gShellEnvVarList.
>
> @param[in] This The pointer to the EFI_FILE_PROTOCOL object.
> @param[in, out] BufferSize Size in bytes of Buffer.
> @param[in] Buffer The pointer to the buffer to write.
>
> - @retval EFI_SUCCESS The data was read.
> + @retval EFI_SUCCESS The data was successfully write to variable.
> + @retval SHELL_OUT_OF_RESOURCES A memory allocation failed.
> **/
> EFI_STATUS
> EFIAPI
> @@ -1146,41 +1177,71 @@ FileInterfaceEnvVolWrite(
> VOID* NewBuffer;
> UINTN NewSize;
> EFI_STATUS Status;
> + UINTN TotalSize;
>
> NewBuffer = NULL;
> NewSize = 0;
> + TotalSize = 0;
>
> Status =
> SHELL_GET_ENVIRONMENT_VARIABLE(((EFI_FILE_PROTOCOL_ENVIRONMENT*
> )This)->Name, &NewSize, NewBuffer);
> - if (Status == EFI_BUFFER_TOO_SMALL){
> - NewBuffer = AllocateZeroPool(NewSize + *BufferSize + sizeof(CHAR16));
> + if (Status == EFI_BUFFER_TOO_SMALL) {
> + TotalSize = NewSize + *BufferSize + sizeof (CHAR16); } else if
> + (Status == EFI_NOT_FOUND) {
> + TotalSize = *BufferSize + sizeof(CHAR16); } else {
> + return Status;
> + }
> +
> + NewBuffer = AllocateZeroPool (TotalSize); if (NewBuffer == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + if (Status == EFI_BUFFER_TOO_SMALL) {
> Status =
> SHELL_GET_ENVIRONMENT_VARIABLE(((EFI_FILE_PROTOCOL_ENVIRONMENT*
> )This)->Name, &NewSize, NewBuffer);
> }
> - if (!EFI_ERROR(Status) && NewBuffer != NULL) {
> - while (((CHAR16*)NewBuffer)[NewSize/2] == CHAR_NULL) {
> - //
> - // We want to overwrite the CHAR_NULL
> - //
> - NewSize -= 2;
> - }
> - CopyMem((UINT8*)NewBuffer + NewSize + 2, Buffer, *BufferSize);
> - Status =
> SHELL_SET_ENVIRONMENT_VARIABLE_V(((EFI_FILE_PROTOCOL_ENVIRONMEN
> T*)This)->Name, StrSize(NewBuffer), NewBuffer);
> - FreePool(NewBuffer);
> - return (Status);
> - } else {
> - SHELL_FREE_NON_NULL(NewBuffer);
> - return
> (SHELL_SET_ENVIRONMENT_VARIABLE_V(((EFI_FILE_PROTOCOL_ENVIRONME
> NT*)This)->Name, *BufferSize, Buffer));
> +
> + if (EFI_ERROR (Status) && Status != EFI_NOT_FOUND) {
> + FreePool (NewBuffer);
> + return Status;
> + }
> +
> + CopyMem ((UINT8*)NewBuffer + NewSize, Buffer, *BufferSize); Status =
> + ShellAddEnvVarToList (
> + ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
> + NewBuffer,
> + TotalSize,
> + EFI_VARIABLE_BOOTSERVICE_ACCESS
> + );
> + if (EFI_ERROR(Status)) {
> + FreePool (NewBuffer);
> + return Status;
> + }
> +
> + Status = SHELL_SET_ENVIRONMENT_VARIABLE_V (
> + ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
> + TotalSize - sizeof (CHAR16),
> + NewBuffer
> + );
> + if (EFI_ERROR(Status)) {
> + ShellRemvoeEnvVarFromList
> + (((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name);
> }
> +
> + FreePool (NewBuffer);
> + return Status;
> }
>
>
> /**
> File style interface for Non Volatile Environment Variable (Write).
> + This function also caches the environment variable into gShellEnvVarList.
>
> @param[in] This The pointer to the EFI_FILE_PROTOCOL object.
> @param[in, out] BufferSize Size in bytes of Buffer.
> @param[in] Buffer The pointer to the buffer to write.
>
> - @retval EFI_SUCCESS The data was read.
> + @retval EFI_SUCCESS The data was successfully write to variable.
> + @retval SHELL_OUT_OF_RESOURCES A memory allocation failed.
> **/
> EFI_STATUS
> EFIAPI
> @@ -1193,27 +1254,58 @@ FileInterfaceEnvNonVolWrite(
> VOID* NewBuffer;
> UINTN NewSize;
> EFI_STATUS Status;
> + UINTN TotalSize;
>
> NewBuffer = NULL;
> NewSize = 0;
> + TotalSize = 0;
>
> Status =
> SHELL_GET_ENVIRONMENT_VARIABLE(((EFI_FILE_PROTOCOL_ENVIRONMENT*
> )This)->Name, &NewSize, NewBuffer);
> - if (Status == EFI_BUFFER_TOO_SMALL){
> - NewBuffer = AllocateZeroPool(NewSize + *BufferSize);
> + if (Status == EFI_BUFFER_TOO_SMALL) {
> + TotalSize = NewSize + *BufferSize + sizeof (CHAR16); } else if
> + (Status == EFI_NOT_FOUND) {
> + TotalSize = *BufferSize + sizeof (CHAR16); } else {
> + return Status;
> + }
> +
> + NewBuffer = AllocateZeroPool (TotalSize); if (NewBuffer == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + if (Status == EFI_BUFFER_TOO_SMALL) {
> Status =
> SHELL_GET_ENVIRONMENT_VARIABLE(((EFI_FILE_PROTOCOL_ENVIRONMENT*
> )This)->Name, &NewSize, NewBuffer);
> }
> - if (!EFI_ERROR(Status)) {
> - CopyMem((UINT8*)NewBuffer + NewSize, Buffer, *BufferSize);
> - return (SHELL_SET_ENVIRONMENT_VARIABLE_NV(
> - ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
> - NewSize + *BufferSize,
> - NewBuffer));
> - } else {
> - return (SHELL_SET_ENVIRONMENT_VARIABLE_NV(
> - ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
> - *BufferSize,
> - Buffer));
> +
> + if (EFI_ERROR(Status) && Status != EFI_NOT_FOUND) {
> + FreePool (NewBuffer);
> + return Status;
> + }
> +
> + CopyMem ((UINT8*) NewBuffer + NewSize, Buffer, *BufferSize); Status
> + = ShellAddEnvVarToList (
> + ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
> + NewBuffer,
> + TotalSize,
> + EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
> + );
> + if (EFI_ERROR (Status)) {
> + FreePool (NewBuffer);
> + return Status;
> }
> +
> + Status = SHELL_SET_ENVIRONMENT_VARIABLE_NV (
> + ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name,
> + TotalSize - sizeof (CHAR16),
> + NewBuffer
> + );
> + if (EFI_ERROR (Status)) {
> + ShellRemvoeEnvVarFromList
> + (((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name);
> + }
> +
> + FreePool (NewBuffer);
> + return Status;
> }
>
> /**
> diff --git a/ShellPkg/Application/Shell/ShellEnvVar.c
> b/ShellPkg/Application/Shell/ShellEnvVar.c
> index 6d8c79b..4c49c1c 100644
> --- a/ShellPkg/Application/Shell/ShellEnvVar.c
> +++ b/ShellPkg/Application/Shell/ShellEnvVar.c
> @@ -178,7 +178,10 @@ GetEnvironmentVariableList(
> Status = EFI_OUT_OF_RESOURCES;
> } else {
> ValSize = ValBufferSize;
> - VarList->Val = AllocateZeroPool(ValSize);
> + //
> + // We need another CHAR16 to save '\0' in VarList->Val.
> + //
> + VarList->Val = AllocateZeroPool (ValSize + sizeof (CHAR16));
> if (VarList->Val == NULL) {
> SHELL_FREE_NON_NULL(VarList);
> Status = EFI_OUT_OF_RESOURCES; @@ -188,7 +191,10 @@
> GetEnvironmentVariableList(
> if (Status == EFI_BUFFER_TOO_SMALL){
> ValBufferSize = ValSize > ValBufferSize * 2 ? ValSize : ValBufferSize * 2;
> SHELL_FREE_NON_NULL (VarList->Val);
> - VarList->Val = AllocateZeroPool(ValBufferSize);
> + //
> + // We need another CHAR16 to save '\0' in VarList->Val.
> + //
> + VarList->Val = AllocateZeroPool (ValBufferSize + sizeof
> + (CHAR16));
> if (VarList->Val == NULL) {
> SHELL_FREE_NON_NULL(VarList);
> Status = EFI_OUT_OF_RESOURCES; @@ -272,7 +278,7 @@
> SetEnvironmentVariableList(
> ; !IsNull(ListHead, &Node->Link)
> ; Node = (ENV_VAR_LIST*)GetNextNode(ListHead, &Node->Link)
> ){
> - Size = StrSize(Node->Val);
> + Size = StrSize (Node->Val) - sizeof (CHAR16);
> if (Node->Atts & EFI_VARIABLE_NON_VOLATILE) {
> Status = SHELL_SET_ENVIRONMENT_VARIABLE_NV(Node->Key, Size, Node-
> >Val);
> } else {
> diff --git a/ShellPkg/Application/Shell/ShellProtocol.c
> b/ShellPkg/Application/Shell/ShellProtocol.c
> index 04b66c5..12c7c40 100644
> --- a/ShellPkg/Application/Shell/ShellProtocol.c
> +++ b/ShellPkg/Application/Shell/ShellProtocol.c
> @@ -2872,8 +2872,8 @@ InternalEfiShellSetEnv(
> );
> if (!EFI_ERROR (Status)) {
> Status = Volatile
> - ? SHELL_SET_ENVIRONMENT_VARIABLE_V(Name, StrSize(Value), Value)
> - : SHELL_SET_ENVIRONMENT_VARIABLE_NV(Name, StrSize(Value),
> Value);
> + ? SHELL_SET_ENVIRONMENT_VARIABLE_V (Name, StrSize (Value) -
> sizeof (CHAR16), Value)
> + : SHELL_SET_ENVIRONMENT_VARIABLE_NV (Name, StrSize (Value)
> + - sizeof (CHAR16), Value);
> if (EFI_ERROR (Status)) {
> ShellRemvoeEnvVarFromList(Name);
> }
> --
> 2.9.0.windows.1
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-12-08 21:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-08 1:45 [PATCH] ShellPkg/Application: Fix ">v" cannot update environment variable Ruiyu Ni
2016-12-08 17:03 ` Shah, Tapan
2016-12-08 21:59 ` Carsey, Jaben
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox