libpldmresponder: Improving Logs (lg2)
This commit corrects the severity level of logs and also formats the
message string, fixing the ill-defined message string of the logs as
mentioned in the anti-pattern document [1]. Additionally, based on the
requirement this commit adds more debug information to logs.
[1]: https://github.com/openbmc/docs/blob/master/anti-patterns.md#ill-defined-data-structuring-in-lg2-message-strings
Change-Id: I7dc5c308a8cd76573995e07d01d1a6037bca31ba
Signed-off-by: Riya Dixit <riyadixitagra@gmail.com>
diff --git a/libpldmresponder/bios_config.cpp b/libpldmresponder/bios_config.cpp
index c45cd3e..699e0d5 100644
--- a/libpldmresponder/bios_config.cpp
+++ b/libpldmresponder/bios_config.cpp
@@ -528,8 +528,8 @@
}
catch (const std::exception& e)
{
- error("failed to update BaseBIOSTable property, ERROR={ERR_EXCEP}",
- "ERR_EXCEP", e.what());
+ error("Failed to update BaseBIOSTable property, error - {ERROR}",
+ "ERROR", e);
}
}
@@ -578,8 +578,8 @@
// bios-settings-manager in sync
catch (const std::exception& e)
{
- error("Failed to read BaseBIOSTable property, ERROR={ERR_EXCEP}",
- "ERR_EXCEP", e.what());
+ error("Failed to read BaseBIOSTable property, error - {ERROR}", "ERROR",
+ e);
}
Table attrTable, attrValueTable;
@@ -604,8 +604,9 @@
}
catch (const std::exception& e)
{
- error("Error constructing table entry for '{ATTR}': {ERROR}",
- "ATTR", attr->name, "ERROR", e);
+ error(
+ "Failed to construct table entry for attribute '{ATTRIBUTE}', error - {ERROR}",
+ "ATTRIBUTE", attr->name, "ERROR", e);
}
}
@@ -688,15 +689,15 @@
catch (const std::exception& e)
{
error(
- "Failed to parse JSON config file(entry handler) : {JSON_PATH}, {ERR_EXCEP}",
- "JSON_PATH", filePath.c_str(), "ERR_EXCEP", e.what());
+ "Failed to parse JSON config file at path '{PATH}', error - {ERROR}",
+ "PATH", filePath.c_str(), "ERROR", e);
}
}
}
catch (const std::exception& e)
{
- error("Failed to parse JSON config at '{PATH}': {ERROR}", "PATH",
- filePath.c_str(), "ERROR", e);
+ error("Failed to parse JSON config file '{PATH}', error - {ERROR}",
+ "PATH", filePath.c_str(), "ERROR", e);
}
}
}
@@ -726,8 +727,8 @@
if (rc != PLDM_SUCCESS)
{
error(
- "Failed to decode BIOS table possible values for attribute entry: {LIPBLDM_ERROR}",
- "LIBPLDM_ERROR", rc);
+ "Failed to decode BIOS table possible values for attribute entry, response code '{RC}'",
+ "RC", rc);
throw std::runtime_error(
"Failed to decode BIOS table possible values for attribute entry");
}
@@ -778,8 +779,8 @@
stringTable);
auto chkBMC = isBMC ? "true" : "false";
info(
- "BIOS: {ATTR_NAME}, updated to value: {NEW_VAL}, by BMC: {CHK_BMC} ",
- "ATTR_NAME", attrName, "NEW_VAL", nwVal, "CHK_BMC", chkBMC);
+ "BIOS attribute '{ATTRIBUTE}' updated to value '{VALUE}' by BMC '{CHECK_BMC}'",
+ "ATTRIBUTE", attrName, "VALUE", nwVal, "CHECK_BMC", chkBMC);
}
break;
}
@@ -790,8 +791,8 @@
table::attribute_value::decodeIntegerEntry(attrValueEntry);
auto chkBMC = isBMC ? "true" : "false";
info(
- "BIOS: {ATTR_NAME}, updated to value: {UPDATED_VAL}, by BMC: {CHK_BMC}",
- "ATTR_NAME", attrName, "UPDATED_VAL", value, "CHK_BMC", chkBMC);
+ "BIOS attribute '{ATTRIBUTE}' updated to value '{VALUE}' by BMC '{CHECK_BMC}'",
+ "ATTRIBUTE", attrName, "VALUE", value, "CHECK_BMC", chkBMC);
break;
}
case PLDM_BIOS_STRING:
@@ -801,8 +802,8 @@
table::attribute_value::decodeStringEntry(attrValueEntry);
auto chkBMC = isBMC ? "true" : "false";
info(
- "BIOS: {ATTR_NAME}, updated to value: {UPDATED_VAL}, by BMC: {CHK_BMC}",
- "ATTR_NAME", attrName, "UPDATED_VAL", value, "CHK_BMC", chkBMC);
+ "BIOS attribute '{ATTRIBUTE}' updated to value '{VALUE}' by BMC '{CHECK_BMC}'",
+ "ATTRIBUTE", attrName, "VALUE", value, "CHECK_BMC", chkBMC);
break;
}
default:
@@ -833,8 +834,9 @@
}
if (value[0] >= pvHdls.size())
{
- error("Enum: Illgeal index, Index = {ATTR_INDEX}", "ATTR_INDEX",
- (int)value[0]);
+ error(
+ "Invalid index '{INDEX}' encountered for Enum type BIOS attribute",
+ "INDEX", (int)value[0]);
return PLDM_ERROR_INVALID_DATA;
}
return PLDM_SUCCESS;
@@ -849,8 +851,10 @@
if (value < lower || value > upper)
{
- error("Integer: out of bound, value = {ATTR_VALUE}",
- "ATTR_VALUE", value);
+ error(
+ "Out of range index '{ATTRIBUTE_VALUE}' encountered for Integer type BIOS attribute for the lower bound '{LOWER}', the upper bound '{UPPER}' and the scalar value '{SCALAR}'.",
+ "ATTRIBUTE_VALUE", value, "LOWER", lower, "UPPER", upper,
+ "SCALAR", scalar);
return PLDM_ERROR_INVALID_DATA;
}
return PLDM_SUCCESS;
@@ -865,15 +869,15 @@
value.size() > stringConf.maxLength)
{
error(
- "String: Length error, string = {ATTR_VALUE} length {LEN}",
- "ATTR_VALUE", value, "LEN", value.size());
+ "Invalid length '{LENGTH}' encountered for string type BIOS attribute value '{ATTRIBUTE_VALUE}' when minimum string entry length '{MIN_LEN}' and maximum string entry length '{MAX_LEN}'",
+ "ATTRIBUTE_VALUE", value, "LENGTH", value.size(), "MIN_LEN",
+ stringConf.minLength, "MAX_LEN", stringConf.maxLength);
return PLDM_ERROR_INVALID_LENGTH;
}
return PLDM_SUCCESS;
}
default:
- error("ReadOnly or Unspported type, type = {ATTR_TYPE}",
- "ATTR_TYPE", attrType);
+ error("ReadOnly or Unsupported type '{TYPE}'", "TYPE", attrType);
return PLDM_ERROR;
};
}
@@ -937,7 +941,7 @@
}
catch (const std::exception& e)
{
- error("Set attribute value error: {ERR_EXCEP}", "ERR_EXCEP", e.what());
+ error("Set attribute value error - {ERROR}", "ERROR", e);
return PLDM_ERROR;
}
@@ -958,7 +962,7 @@
}
catch (const std::exception& e)
{
- error("Remove the tables error: {ERR_EXCEP}", "ERR_EXCEP", e.what());
+ error("Remove the tables error - {ERROR}", "ERROR", e);
}
}
@@ -990,15 +994,16 @@
}
catch (const std::invalid_argument& e)
{
- error("Missing handle for '{ATTR}': {ERROR}", "ATTR", attrName, "ERROR",
- e);
+ error(
+ "Missing handle for attribute '{ATTRIBUTE}' in BIOS String Table, error - '{ERROR}'",
+ "ATTRIBUTE", attrName, "ERROR", e);
return;
}
auto attrTable = getBIOSTable(PLDM_BIOS_ATTR_TABLE);
if (!attrTable.has_value())
{
- error("Attribute table not present");
+ error("BIOS Attribute table not present");
return;
}
const struct pldm_bios_attr_table_entry* tableEntry =
@@ -1006,8 +1011,8 @@
if (tableEntry == nullptr)
{
error(
- "Attribute not found in attribute table, name= {ATTR_NAME} name handle={ATTR_HANDLE}",
- "ATTR_NAME", attrName.c_str(), "ATTR_HANDLE", attrNameHdl);
+ "Failed to find attribute {ATTRIBUTE} in BIOS Attribute table with attribute handle '{ATTR_HANDLE}'",
+ "ATTRIBUTE", attrName, "ATTR_HANDLE", attrNameHdl);
return;
}
@@ -1028,8 +1033,8 @@
if (rc != PLDM_SUCCESS)
{
error(
- "Could not update the attribute value table for attribute handle={ATTR_HANDLE} and type={ATTR_TYPE}",
- "ATTR_HANDLE", attrHdl, "ATTR_TYPE", (uint32_t)attrType);
+ "Failed to update the attribute value table for attribute handle '{ATTR_HANDLE}' and attribute type '{TYPE}'",
+ "ATTR_HANDLE", attrHdl, "TYPE", (uint32_t)attrType);
return;
}
auto destTable = table::attribute_value::updateTable(
@@ -1042,8 +1047,9 @@
rc = setAttrValue(newValue.data(), newValue.size(), true, false);
if (rc != PLDM_SUCCESS)
{
- error("could not setAttrValue on base bios table and dbus, rc = {RC}",
- "RC", rc);
+ error(
+ "Failed to setAttrValue on base bios table and dbus, response code '{RC}'",
+ "RC", rc);
}
}
@@ -1087,8 +1093,7 @@
if (iter == biosAttributes.end())
{
- error("Wrong attribute name, attributeName = {ATTR_NAME}",
- "ATTR_NAME", attributeName);
+ error("Wrong attribute name {NAME}", "NAME", attributeName);
continue;
}
@@ -1104,8 +1109,8 @@
type != BIOSConfigManager::AttributeType::String &&
type != BIOSConfigManager::AttributeType::Integer)
{
- error("Attribute type not supported, attributeType = {ATTR_TYPE}",
- "ATTR_TYPE", attributeType);
+ error("Attribute type '{TYPE}' not supported", "TYPE",
+ attributeType);
continue;
}