[edk2] [PATCH v2] MdeModulePkg: Fix a error that return type differs from the left one when assigned

[Zhang Lubo]

Message-ID: <46458795-305f-4947-bf39-***@LUBOZHAN-MOBL.local>
X-Mailer: TortoiseGit
MIME-Version: 1.0
Content-Type: text/plain;

Version v2 includes using local variable to save intermediate variable to make codes easier readable.
Using ASSERT_EFI_ERROR instead of check the return status.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo <***@intel.com>
---
.../Universal/Network/Mtftp4Dxe/Mtftp4Support.c | 61 +++++++++++++---------
1 file changed, 37 insertions(+), 24 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
index 647fe64..c625fda 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
@@ -276,17 +276,20 @@ Mtftp4SendRequest (
)
{
EFI_MTFTP4_PACKET *Packet;
EFI_MTFTP4_OPTION *Options;
EFI_MTFTP4_TOKEN *Token;
+ RETURN_STATUS Status;
NET_BUF *Nbuf;
UINT8 *Mode;
UINT8 *Cur;
- UINT32 Len;
UINTN Index;
- UINT32 Len1;
- UINT32 Len2;
+ UINT32 BufferLength;
+ UINTN FileNameLength;
+ UINTN ModeLength;
+ UINTN OptionStrLength;
+ UINTN ValueStrLength;

Token = Instance->Token;
Options = Token->OptionList;
Mode = Instance->Token->ModeStr;

@@ -295,46 +298,56 @@ Mtftp4SendRequest (
}

//
// Compute the packet length
//
- Len1 = (UINT32) AsciiStrLen ((CHAR8 *) Token->Filename);
- Len2 = (UINT32) AsciiStrLen ((CHAR8 *) Mode);
- Len = (Len1 + Len2 + 4);
+ FileNameLength = AsciiStrLen ((CHAR8 *) Token->Filename);
+ ModeLength = AsciiStrLen ((CHAR8 *) Mode);
+ BufferLength = (UINT32) FileNameLength + (UINT32) ModeLength + 4;

for (Index = 0; Index < Token->OptionCount; Index++) {
- Len1 = (UINT32) AsciiStrLen ((CHAR8 *) Options[Index].OptionStr);
- Len2 = (UINT32) AsciiStrLen ((CHAR8 *) Options[Index].ValueStr);
- Len += Len1 + Len2 + 2;
+ OptionStrLength = AsciiStrLen ((CHAR8 *) Options[Index].OptionStr);
+ ValueStrLength = AsciiStrLen ((CHAR8 *) Options[Index].ValueStr);
+ BufferLength += (UINT32) OptionStrLength + (UINT32) ValueStrLength + 2;
}
-
//
// Allocate a packet then copy the data over
//
- if ((Nbuf = NetbufAlloc (Len)) == NULL) {
+ if ((Nbuf = NetbufAlloc (BufferLength)) == NULL) {
return EFI_OUT_OF_RESOURCES;
}

- Packet = (EFI_MTFTP4_PACKET *) NetbufAllocSpace (Nbuf, Len, FALSE);
+ Packet = (EFI_MTFTP4_PACKET *) NetbufAllocSpace (Nbuf, BufferLength, FALSE);
ASSERT (Packet != NULL);

Packet->OpCode = HTONS (Instance->Operation);
+ BufferLength -= sizeof (Packet->OpCode);
+
Cur = Packet->Rrq.Filename;
- Cur = (UINT8 *) AsciiStrCpyS ((CHAR8 *) Cur, Len - 2, (CHAR8 *) Token->Filename);
- Cur += AsciiStrLen ((CHAR8 *) Token->Filename) + 1;
- Cur = (UINT8 *) AsciiStrCpyS ((CHAR8 *) Cur, Len - 2 - (AsciiStrLen ((CHAR8 *) Token->Filename) + 1), (CHAR8 *) Mode);
- Cur += AsciiStrLen ((CHAR8 *) Mode) + 1;
- Len -= ((UINT32) AsciiStrLen ((CHAR8 *) Token->Filename) + (UINT32) AsciiStrLen ((CHAR8 *) Mode) + 4);
+ Status = AsciiStrCpyS ((CHAR8 *) Cur, BufferLength, (CHAR8 *) Token->Filename);
+ ASSERT_EFI_ERROR (Status);
+ BufferLength -= (UINT32) (FileNameLength + 1);
+ Cur += FileNameLength + 1;
+ Status = AsciiStrCpyS ((CHAR8 *) Cur, BufferLength, (CHAR8 *) Mode);
+ ASSERT_EFI_ERROR (Status);
+ BufferLength -= (UINT32) (ModeLength + 1);
+ Cur += ModeLength + 1;

for (Index = 0; Index < Token->OptionCount; ++Index) {
- Cur = (UINT8 *) AsciiStrCpyS ((CHAR8 *) Cur, Len, (CHAR8 *) Options[Index].OptionStr);
- Cur += AsciiStrLen ((CHAR8 *) Options[Index].OptionStr) + 1;
- Len -= (UINT32) AsciiStrLen ((CHAR8 *) Options[Index].OptionStr) + 1;
-
- Cur = (UINT8 *) AsciiStrCpyS ((CHAR8 *) Cur, Len, (CHAR8 *) Options[Index].ValueStr);
- Cur += AsciiStrLen ((CHAR8 *) (CHAR8 *) Options[Index].ValueStr) + 1;
- Len -= (UINT32) AsciiStrLen ((CHAR8 *) (CHAR8 *) Options[Index].ValueStr) + 1;
+ OptionStrLength = AsciiStrLen ((CHAR8 *) Options[Index].OptionStr);
+ ValueStrLength = AsciiStrLen ((CHAR8 *) Options[Index].ValueStr);
+
+ Status = AsciiStrCpyS ((CHAR8 *) Cur, BufferLength, (CHAR8 *) Options[Index].OptionStr);
+ ASSERT_EFI_ERROR (Status);
+ BufferLength -= (UINT32) (OptionStrLength + 1);
+ Cur += OptionStrLength + 1;
+
+ Status = AsciiStrCpyS ((CHAR8 *) Cur, BufferLength, (CHAR8 *) Options[Index].ValueStr);
+ ASSERT_EFI_ERROR (Status);
+ BufferLength -= (UINT32) (ValueStrLength + 1);
+ Cur += ValueStrLength + 1;
+
}

return Mtftp4SendPacket (Instance, Nbuf);
}

[Fu, Siyuan]

Reviewed-by: Fu Siyuan <***@intel.com>

-----Original Message-----
From: Zhang, Lubo
Sent: Friday, July 10, 2015 2:11 PM
To: edk2-***@lists.sourceforge.net; Fu, Siyuan; Qiu, Shumin; Wu, Jiaxin
Subject: [PATCH v2] MdeModulePkg: Fix a error that return type differs from the left one when assigned

Message-ID: <46458795-305f-4947-bf39-***@LUBOZHAN-MOBL.local>
X-Mailer: TortoiseGit
MIME-Version: 1.0
Content-Type: text/plain;

Version v2 includes using local variable to save intermediate variable to make codes easier readable.
Using ASSERT_EFI_ERROR instead of check the return status.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo <***@intel.com>
---
.../Universal/Network/Mtftp4Dxe/Mtftp4Support.c | 61 +++++++++++++---------
1 file changed, 37 insertions(+), 24 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
index 647fe64..c625fda 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
@@ -276,17 +276,20 @@ Mtftp4SendRequest (
)
{
EFI_MTFTP4_PACKET *Packet;
EFI_MTFTP4_OPTION *Options;
EFI_MTFTP4_TOKEN *Token;
+ RETURN_STATUS Status;
NET_BUF *Nbuf;
UINT8 *Mode;
UINT8 *Cur;
- UINT32 Len;
UINTN Index;
- UINT32 Len1;
- UINT32 Len2;
+ UINT32 BufferLength;
+ UINTN FileNameLength;
+ UINTN ModeLength;
+ UINTN OptionStrLength;
+ UINTN ValueStrLength;

Token = Instance->Token;
Options = Token->OptionList;
Mode = Instance->Token->ModeStr;

@@ -295,46 +298,56 @@ Mtftp4SendRequest (
}

//
// Compute the packet length
//
- Len1 = (UINT32) AsciiStrLen ((CHAR8 *) Token->Filename);
- Len2 = (UINT32) AsciiStrLen ((CHAR8 *) Mode);
- Len = (Len1 + Len2 + 4);
+ FileNameLength = AsciiStrLen ((CHAR8 *) Token->Filename);
+ ModeLength = AsciiStrLen ((CHAR8 *) Mode);
+ BufferLength = (UINT32) FileNameLength + (UINT32) ModeLength + 4;

for (Index = 0; Index < Token->OptionCount; Index++) {
- Len1 = (UINT32) AsciiStrLen ((CHAR8 *) Options[Index].OptionStr);
- Len2 = (UINT32) AsciiStrLen ((CHAR8 *) Options[Index].ValueStr);
- Len += Len1 + Len2 + 2;
+ OptionStrLength = AsciiStrLen ((CHAR8 *) Options[Index].OptionStr);
+ ValueStrLength = AsciiStrLen ((CHAR8 *) Options[Index].ValueStr);
+ BufferLength += (UINT32) OptionStrLength + (UINT32) ValueStrLength + 2;
}
-
//
// Allocate a packet then copy the data over
//
- if ((Nbuf = NetbufAlloc (Len)) == NULL) {
+ if ((Nbuf = NetbufAlloc (BufferLength)) == NULL) {
return EFI_OUT_OF_RESOURCES;
}

- Packet = (EFI_MTFTP4_PACKET *) NetbufAllocSpace (Nbuf, Len, FALSE);
+ Packet = (EFI_MTFTP4_PACKET *) NetbufAllocSpace (Nbuf, BufferLength, FALSE);
ASSERT (Packet != NULL);

Packet->OpCode = HTONS (Instance->Operation);
+ BufferLength -= sizeof (Packet->OpCode);
+
Cur = Packet->Rrq.Filename;
- Cur = (UINT8 *) AsciiStrCpyS ((CHAR8 *) Cur, Len - 2, (CHAR8 *) Token->Filename);
- Cur += AsciiStrLen ((CHAR8 *) Token->Filename) + 1;
- Cur = (UINT8 *) AsciiStrCpyS ((CHAR8 *) Cur, Len - 2 - (AsciiStrLen ((CHAR8 *) Token->Filename) + 1), (CHAR8 *) Mode);
- Cur += AsciiStrLen ((CHAR8 *) Mode) + 1;
- Len -= ((UINT32) AsciiStrLen ((CHAR8 *) Token->Filename) + (UINT32) AsciiStrLen ((CHAR8 *) Mode) + 4);
+ Status = AsciiStrCpyS ((CHAR8 *) Cur, BufferLength, (CHAR8 *) Token->Filename);
+ ASSERT_EFI_ERROR (Status);
+ BufferLength -= (UINT32) (FileNameLength + 1);
+ Cur += FileNameLength + 1;
+ Status = AsciiStrCpyS ((CHAR8 *) Cur, BufferLength, (CHAR8 *) Mode);
+ ASSERT_EFI_ERROR (Status);
+ BufferLength -= (UINT32) (ModeLength + 1);
+ Cur += ModeLength + 1;

for (Index = 0; Index < Token->OptionCount; ++Index) {
- Cur = (UINT8 *) AsciiStrCpyS ((CHAR8 *) Cur, Len, (CHAR8 *) Options[Index].OptionStr);
- Cur += AsciiStrLen ((CHAR8 *) Options[Index].OptionStr) + 1;
- Len -= (UINT32) AsciiStrLen ((CHAR8 *) Options[Index].OptionStr) + 1;
-
- Cur = (UINT8 *) AsciiStrCpyS ((CHAR8 *) Cur, Len, (CHAR8 *) Options[Index].ValueStr);
- Cur += AsciiStrLen ((CHAR8 *) (CHAR8 *) Options[Index].ValueStr) + 1;
- Len -= (UINT32) AsciiStrLen ((CHAR8 *) (CHAR8 *) Options[Index].ValueStr) + 1;
+ OptionStrLength = AsciiStrLen ((CHAR8 *) Options[Index].OptionStr);
+ ValueStrLength = AsciiStrLen ((CHAR8 *) Options[Index].ValueStr);
+
+ Status = AsciiStrCpyS ((CHAR8 *) Cur, BufferLength, (CHAR8 *) Options[Index].OptionStr);
+ ASSERT_EFI_ERROR (Status);
+ BufferLength -= (UINT32) (OptionStrLength + 1);
+ Cur += OptionStrLength + 1;
+
+ Status = AsciiStrCpyS ((CHAR8 *) Cur, BufferLength, (CHAR8 *) Options[Index].ValueStr);
+ ASSERT_EFI_ERROR (Status);
+ BufferLength -= (UINT32) (ValueStrLength + 1);
+ Cur += ValueStrLength + 1;
+
}

return Mtftp4SendPacket (Instance, Nbuf); }

--
1.9.5.msysgit.1

[Wu, Jiaxin]

Reviewed-by: Jiaxin Wu <***@intel.com>


-----Original Message-----
From: Zhang, Lubo
Sent: Friday, July 10, 2015 2:11 PM
To: edk2-***@lists.sourceforge.net; Fu, Siyuan; Qiu, Shumin; Wu, Jiaxin
Subject: [PATCH v2] MdeModulePkg: Fix a error that return type differs from the left one when assigned

Message-ID: <46458795-305f-4947-bf39-***@LUBOZHAN-MOBL.local>
X-Mailer: TortoiseGit
MIME-Version: 1.0
Content-Type: text/plain;

Version v2 includes using local variable to save intermediate variable to make codes easier readable.
Using ASSERT_EFI_ERROR instead of check the return status.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo <***@intel.com>
---
.../Universal/Network/Mtftp4Dxe/Mtftp4Support.c | 61 +++++++++++++---------
1 file changed, 37 insertions(+), 24 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
index 647fe64..c625fda 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
@@ -276,17 +276,20 @@ Mtftp4SendRequest (
)
{
EFI_MTFTP4_PACKET *Packet;
EFI_MTFTP4_OPTION *Options;
EFI_MTFTP4_TOKEN *Token;
+ RETURN_STATUS Status;
NET_BUF *Nbuf;
UINT8 *Mode;
UINT8 *Cur;
- UINT32 Len;
UINTN Index;
- UINT32 Len1;
- UINT32 Len2;
+ UINT32 BufferLength;
+ UINTN FileNameLength;
+ UINTN ModeLength;
+ UINTN OptionStrLength;
+ UINTN ValueStrLength;

Token = Instance->Token;
Options = Token->OptionList;
Mode = Instance->Token->ModeStr;

@@ -295,46 +298,56 @@ Mtftp4SendRequest (
}

//
// Compute the packet length
//
- Len1 = (UINT32) AsciiStrLen ((CHAR8 *) Token->Filename);
- Len2 = (UINT32) AsciiStrLen ((CHAR8 *) Mode);
- Len = (Len1 + Len2 + 4);
+ FileNameLength = AsciiStrLen ((CHAR8 *) Token->Filename);
+ ModeLength = AsciiStrLen ((CHAR8 *) Mode);
+ BufferLength = (UINT32) FileNameLength + (UINT32) ModeLength + 4;

for (Index = 0; Index < Token->OptionCount; Index++) {
- Len1 = (UINT32) AsciiStrLen ((CHAR8 *) Options[Index].OptionStr);
- Len2 = (UINT32) AsciiStrLen ((CHAR8 *) Options[Index].ValueStr);
- Len += Len1 + Len2 + 2;
+ OptionStrLength = AsciiStrLen ((CHAR8 *) Options[Index].OptionStr);
+ ValueStrLength = AsciiStrLen ((CHAR8 *) Options[Index].ValueStr);
+ BufferLength += (UINT32) OptionStrLength + (UINT32) ValueStrLength + 2;
}
-
//
// Allocate a packet then copy the data over
//
- if ((Nbuf = NetbufAlloc (Len)) == NULL) {
+ if ((Nbuf = NetbufAlloc (BufferLength)) == NULL) {
return EFI_OUT_OF_RESOURCES;
}

- Packet = (EFI_MTFTP4_PACKET *) NetbufAllocSpace (Nbuf, Len, FALSE);
+ Packet = (EFI_MTFTP4_PACKET *) NetbufAllocSpace (Nbuf, BufferLength, FALSE);
ASSERT (Packet != NULL);

Packet->OpCode = HTONS (Instance->Operation);
+ BufferLength -= sizeof (Packet->OpCode);
+
Cur = Packet->Rrq.Filename;
- Cur = (UINT8 *) AsciiStrCpyS ((CHAR8 *) Cur, Len - 2, (CHAR8 *) Token->Filename);
- Cur += AsciiStrLen ((CHAR8 *) Token->Filename) + 1;
- Cur = (UINT8 *) AsciiStrCpyS ((CHAR8 *) Cur, Len - 2 - (AsciiStrLen ((CHAR8 *) Token->Filename) + 1), (CHAR8 *) Mode);
- Cur += AsciiStrLen ((CHAR8 *) Mode) + 1;
- Len -= ((UINT32) AsciiStrLen ((CHAR8 *) Token->Filename) + (UINT32) AsciiStrLen ((CHAR8 *) Mode) + 4);
+ Status = AsciiStrCpyS ((CHAR8 *) Cur, BufferLength, (CHAR8 *) Token->Filename);
+ ASSERT_EFI_ERROR (Status);
+ BufferLength -= (UINT32) (FileNameLength + 1);
+ Cur += FileNameLength + 1;
+ Status = AsciiStrCpyS ((CHAR8 *) Cur, BufferLength, (CHAR8 *) Mode);
+ ASSERT_EFI_ERROR (Status);
+ BufferLength -= (UINT32) (ModeLength + 1);
+ Cur += ModeLength + 1;

for (Index = 0; Index < Token->OptionCount; ++Index) {
- Cur = (UINT8 *) AsciiStrCpyS ((CHAR8 *) Cur, Len, (CHAR8 *) Options[Index].OptionStr);
- Cur += AsciiStrLen ((CHAR8 *) Options[Index].OptionStr) + 1;
- Len -= (UINT32) AsciiStrLen ((CHAR8 *) Options[Index].OptionStr) + 1;
-
- Cur = (UINT8 *) AsciiStrCpyS ((CHAR8 *) Cur, Len, (CHAR8 *) Options[Index].ValueStr);
- Cur += AsciiStrLen ((CHAR8 *) (CHAR8 *) Options[Index].ValueStr) + 1;
- Len -= (UINT32) AsciiStrLen ((CHAR8 *) (CHAR8 *) Options[Index].ValueStr) + 1;
+ OptionStrLength = AsciiStrLen ((CHAR8 *) Options[Index].OptionStr);
+ ValueStrLength = AsciiStrLen ((CHAR8 *) Options[Index].ValueStr);
+
+ Status = AsciiStrCpyS ((CHAR8 *) Cur, BufferLength, (CHAR8 *) Options[Index].OptionStr);
+ ASSERT_EFI_ERROR (Status);
+ BufferLength -= (UINT32) (OptionStrLength + 1);
+ Cur += OptionStrLength + 1;
+
+ Status = AsciiStrCpyS ((CHAR8 *) Cur, BufferLength, (CHAR8 *) Options[Index].ValueStr);
+ ASSERT_EFI_ERROR (Status);
+ BufferLength -= (UINT32) (ValueStrLength + 1);
+ Cur += ValueStrLength + 1;
+
}

return Mtftp4SendPacket (Instance, Nbuf); }

--
1.9.5.msysgit.1