Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

python vfrcompiler tool #102

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
83 changes: 83 additions & 0 deletions edk2basetools/VfrCompiler/IfrCommon.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
from edk2basetools.VfrCompiler.IfrCtypes import *
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • imports are dangerous as you import more then you mean to. As an example you are also importing Ed2Logger, where you should actually have a separate import for EdkLogger


# Enumeration of EFI_STATUS.
RETURN_SUCCESS = EFI_SUCCESS = 0
EFI_BUFFER_TOO_SMALL = 0x8000000000000000 | (5)
EFI_ABORTED = 0x8000000000000000 | (21)
EFI_OUT_OF_RESOURCES = 0x8000000000000000 | (9)
EFI_INVALID_PARAMETER = 0x8000000000000000 | (2)
EFI_NOT_FOUND = 0x8000000000000000 | (14)
RETURN_INVALID_PARAMETER = 0x8000000000000000 | (2)
RETURN_UNSUPPORTED = 0x8000000000000000 | (3)


def EFI_ERROR(A):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing return type annotation

Suggested change
def EFI_ERROR(A):
def EFI_ERROR(A) -> bool:

if A < 0:
return True
else:
return False
Comment on lines +15 to +18
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if A < 0:
return True
else:
return False
return A < 0



# Converts a string to an EFI_GUID.
def StringToGuid(AsciiGuidBuffer: str, GuidBuffer: EFI_GUID):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing return type annotation

Data4 = [0] * 8
if AsciiGuidBuffer == None or GuidBuffer == None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When comparing values to None, one should always do cond is None

Suggested change
if AsciiGuidBuffer == None or GuidBuffer == None:
if AsciiGuidBuffer is None or GuidBuffer is None:

return EFI_INVALID_PARAMETER
Index = 0
while Index < 36:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a constant variable rather than a magic value.

if Index == 8 or Index == 13 or Index == 18 or Index == 23:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a constant variable rather than a magic value.

if AsciiGuidBuffer[Index] != "-":
break
else:
if (
Comment on lines +31 to +32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be refactored to be an elif and then else statement

(AsciiGuidBuffer[Index] >= "0" and AsciiGuidBuffer[Index] <= "9")
or (AsciiGuidBuffer[Index] >= "a" and AsciiGuidBuffer[Index] <= "f")
or (AsciiGuidBuffer[Index] >= "A" and AsciiGuidBuffer[Index] <= "F")
):
Index += 1
continue
else:
break
Index += 1
continue

if Index < 36:
EdkLogger.error("VfrCompiler", PARAMETER_INVALID, "Invalid option value")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edk2Logger should be directly imported in this file.

return EFI_ABORTED
Index = 11
try:
Data1 = int(AsciiGuidBuffer[0:8], 16)
Data2 = int(AsciiGuidBuffer[9:13], 16)
Data3 = int(AsciiGuidBuffer[14:18], 16)
Data4[0] = int(AsciiGuidBuffer[19:21], 16)
Data4[1] = int(AsciiGuidBuffer[21:23], 16)
Data4[2] = int(AsciiGuidBuffer[24:26], 16)
Data4[3] = int(AsciiGuidBuffer[26:28], 16)
Data4[4] = int(AsciiGuidBuffer[28:30], 16)
Data4[5] = int(AsciiGuidBuffer[30:32], 16)
Data4[6] = int(AsciiGuidBuffer[32:34], 16)
Data4[7] = int(AsciiGuidBuffer[34:36], 16)
except:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should never use a bare except statement. This can catch extra exceptions like KeyboardInterrupt and SystemExit which can lock up your code. You should always do except Exception at the bare minimum

EdkLogger.error("VfrCompiler", PARAMETER_INVALID, "Invalid Data value!")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edk2Logger should be directly imported in this file.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Javagedes, thank you very much for your valuable review comments. 😊
We will commit to addressing the issues and propose a new pull request after making all modifications~

Index = 0

# Verify the correct number of items were scanned.
if Index != 11:
EdkLogger.error("VfrCompiler", PARAMETER_INVALID, "Invalid option value")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edk2Logger should be directly imported in this file.

return EFI_ABORTED

# Copy the data into our GUID.
GuidBuffer.Data1 = Data1
GuidBuffer.Data2 = Data2
GuidBuffer.Data3 = Data3
GuidBuffer.Data4[0] = Data4[0]
GuidBuffer.Data4[1] = Data4[1]
GuidBuffer.Data4[2] = Data4[2]
GuidBuffer.Data4[3] = Data4[3]
GuidBuffer.Data4[4] = Data4[4]
GuidBuffer.Data4[5] = Data4[5]
GuidBuffer.Data4[6] = Data4[6]
GuidBuffer.Data4[7] = Data4[7]
Status = EFI_SUCCESS

return Status, GuidBuffer
Loading