On 08/21/2017 09:05 AM, Laszlo Ersek wrote:
[...]
for (Index = 0; Index < RNGValueLength; Index++) {
RNGValue[Index] = Buffer[Index];
}
Status = EFI_SUCCESS;
+ //
+ // Buffer is already Unmaped(), goto free it.
+ //
+ goto FreeBuffer;
(6) We restrict "goto" statements to error handling:
https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/5_source_files/57_c_programming.html
5.7.3.8 Goto Statements should not be used (in general)
[...]
It is common to use goto for error handling and thus, exiting a
routine in an error case is the only legal use of a goto. [...]
So please open-code the FreePool() call and the "return" statement here.
I was wondering if something like this is acceptable ? Actually since we need
to handle
the Unmap due to error from VirtioFlush hence I was trying to minimize the
adding
new state variables or additional checks inside the for loop.
@@ -178,17 +194,34 @@ VirtioRngGetRNG (
if (VirtioFlush (Dev->VirtIo, 0, &Dev->Ring, &Indices, &Len) !=
EFI_SUCCESS) {
Status = EFI_DEVICE_ERROR;
- goto FreeBuffer;
+ goto UnmapBuffer;
}
ASSERT (Len > 0);
ASSERT (Len <= BufferSize);
}
+ //
+ // Unmap the device buffer before accessing it.
+ //
+ Status = Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Mapping);
+ if (EFI_ERROR (Status)) {
+ goto FreeBuffer;
+ }
+
for (Index = 0; Index < RNGValueLength; Index++) {
RNGValue[Index] = Buffer[Index];
}
Status = EFI_SUCCESS;
+UnmapBuffer:
+ //
+ // If we are reached here due to the error then unmap the buffer otherwise
+ // the buffer is already unmapped after VirtioFlush().
+ //
+ if (EFI_ERROR (Status)) {
+ Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Mapping);
+ }
+
FreeBuffer:
FreePool ((VOID *)Buffer);
return Status;
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
