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

RedfishPkg/RedfishPlatformConfigDxe: check attribute max. and min. value #6452

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nicklela
Copy link
Contributor

Description

  • For integer attribute, check and see if its value is between maximum and minimum value defined by HII question.

  • For string attribute, check and see if its string length is between maximum string length and minimum string length defined by HII question.

  • Breaking change?

    • Breaking change - Does this PR cause a break in build or boot behavior?
    • Examples: Does it add a new library class or move a module to a different repo.
  • Impacts security?

    • Security - Does this PR have a direct security impact?
    • Examples: Crypto algorithm change or buffer overflow fix.
  • Includes tests?

    • Tests - Does this PR include any explicit test code?
    • Examples: Unit tests or integration tests.

How This Was Tested

  • Build and test on Arm platform.

Integration Instructions

N/A

- For integer attribute, check and see if its value is between maximum
and minimum value defined by HII question.
- For string attribute, check and see if its string length is between
maximum string length and minimum string length defined by HII question.

Signed-off-by: Nickle Wang <nicklew@nvidia.com>
@nicklela
Copy link
Contributor Author

@igorkulchytskyy could you help me to review this change if you have chance? Thanks!

@igorkulchytskyy
Copy link
Contributor

@igorkulchytskyy could you help me to review this change if you have chance? Thanks!

@nicklela I asked a question about a week ago to clarify one thing. Other than this question, everything looks good for me. Could you please answer my question, so I can finish my review process.
Thank you,
Igor

@nicklela
Copy link
Contributor Author

@igorkulchytskyy could you help me to review this change if you have chance? Thanks!

@nicklela I asked a question about a week ago to clarify one thing. Other than this question, everything looks good for me. Could you please answer my question, so I can finish my review process. Thank you, Igor

Hi Igor, it looks like I cannot see your question on GitHub again. Below is what I see on this page. Can you post your question again? Thanks!

image

Or you are talking about the question in email discussion? If yes, this is separated new review and this is to fix a bug in RedfishPkg. For the question in email, I still need time to finish my diagram of explaining the Redfish task service.

DEBUG ((DEBUG_ERROR, "%a: string length: %u is smaller than minimum string length: %u\n", __func__, StrLength, TargetStatement->StatementData.StrMinSize));
return EFI_ACCESS_DENIED;
}

//
Copy link
Contributor

Choose a reason for hiding this comment

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

Nickle,
what if some statement does not have StrMaxSize set?
Will we have it as 0 and the condition fail?
Or in this case the condition should not be verified?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants