-
Notifications
You must be signed in to change notification settings - Fork 21
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
Keyword VPD:Write keyword's value on hardware #459
base: P11_Dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
#include "exceptions.hpp" | ||
#include "logger.hpp" | ||
|
||
#include <fstream> | ||
#include <iostream> | ||
#include <numeric> | ||
#include <string> | ||
|
@@ -133,4 +134,80 @@ void KeywordVpdParser::checkNextBytesValidity(uint8_t i_numberOfBytes) | |
} | ||
} | ||
|
||
int KeywordVpdParser::findKeyword(const std::string& i_keyword) | ||
{ | ||
(void)i_keyword; | ||
return 0; | ||
} | ||
|
||
int KeywordVpdParser::writeKeywordOnHardware( | ||
const types::WriteVpdParams i_paramsToWriteData) | ||
{ | ||
types::Keyword l_keywordName; | ||
types::BinaryVector l_keywordData; | ||
|
||
// Extract keyword and value from i_paramsToWriteData | ||
if (const types::KwData* l_kwData = | ||
std::get_if<types::KwData>(&i_paramsToWriteData)) | ||
{ | ||
l_keywordName = std::get<0>(*l_kwData); | ||
l_keywordData = std::get<1>(*l_kwData); | ||
} | ||
else | ||
{ | ||
logging::logMessage("Given VPD type is not supported"); | ||
throw types::DbusInvalidArgument(); | ||
} | ||
|
||
if (l_keywordData.size() == 0) | ||
{ | ||
logging::logMessage("Given keyword's data is of length 0"); | ||
throw types::DbusInvalidArgument(); | ||
} | ||
|
||
// Iterate through VPD vector to find the keyword | ||
if (findKeyword(l_keywordName) != 0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where is this defined? Can this be done soon after line 161, so that you don't need to check the keyword data size if this keyword is not a valid one There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The l_keywordData is the user given keyword's data to update on EEPROM. |
||
{ | ||
logging::logMessage("Keyword " + l_keywordName + " not found."); | ||
throw types::DbusInvalidArgument(); | ||
} | ||
|
||
// Skip bytes representing the keyword name | ||
std::advance(m_vpdIterator, constants::TWO_BYTES); | ||
|
||
// Get size of the keyword | ||
const auto l_keywordActualSize = *m_vpdIterator; | ||
|
||
// If given keyword value is of greater length that it's actual length | ||
if (l_keywordData.size() > l_keywordActualSize) | ||
{ | ||
// Resize the given array to the right size | ||
l_keywordData.resize(l_keywordActualSize); | ||
l_keywordData.shrink_to_fit(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How to ensure the shrinking will get the correct value as a result? Will this not corrupt it? Should you throw here as invalid? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. resize will erase the extra contents and resizes the vector to l_keywordActualSize. Here only the contents are erased but the vector capacity doesn't change. |
||
} | ||
|
||
// Skip bytes representing the size of the keyword | ||
std::advance(m_vpdIterator, constants::ONE_BYTE); | ||
|
||
// Open filestream to write the value | ||
std::fstream l_vpdFileStream; | ||
l_vpdFileStream.exceptions(std::ifstream::badbit | std::ifstream::failbit); | ||
l_vpdFileStream.open(m_vpdFilePath, | ||
std::ios::in | std::ios::out | std::ios::binary); | ||
|
||
// Get keyword value's offset | ||
const auto l_keywordOffset = std::distance(m_keywordVpdVector.begin(), | ||
m_vpdIterator); | ||
// Goto keyword value's offset in filestream | ||
l_vpdFileStream.seekp(constants::KW_VPD_DATA_START + l_keywordOffset, | ||
std::ios::beg); | ||
|
||
// Write given value in filestream | ||
std::copy(l_keywordData.begin(), l_keywordData.end(), | ||
std::ostreambuf_iterator<char>(l_vpdFileStream)); | ||
|
||
l_vpdFileStream.close(); | ||
|
||
return l_keywordData.size(); | ||
} | ||
} // namespace vpd |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -120,7 +120,8 @@ std::shared_ptr<ParserInterface> | |
{ | ||
logging::logMessage("KWD vpd parser selected for VPD file path " + | ||
i_vpdFilePath); | ||
return std::make_shared<KeywordVpdParser>(i_vpdVector); | ||
return std::make_shared<KeywordVpdParser>(i_vpdVector, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this file path validated before using ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. validated at the caller's place here https://github.com/ibm-openbmc/openpower-vpd-parser/blob/P11_Dev/src/manager.cpp#L218 . But yeah we have to validate in this API as well so that we are safe even if anyother caller failed to validate. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we will create a new issue to address such validations across the repo. |
||
i_vpdFilePath); | ||
} | ||
|
||
case vpdType::DDR5_DDIMM_MEMORY_VPD: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the possibilities of a keyword value being size 0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User by mistake sending an empty buffer to update on VPD.