From 109901bbeb0ef7b29daa6b5097b1242ac480ffd6 Mon Sep 17 00:00:00 2001 From: jief666 Date: Sat, 30 Oct 2021 08:58:43 +0200 Subject: [PATCH] Better log in case of messages about config,plist. Implememts --info in ccpv. --- .../CloverConfigPlistValidator_Debug.xcscheme | 14 ++- Xcode/CloverConfigPlistValidator/src/main.cpp | 102 +++++++++++++++--- rEFIt_UEFI/Settings/ConfigManager.cpp | 23 ++-- rEFIt_UEFI/Settings/ConfigPlist/SMBIOSPlist.h | 64 ++++++----- rEFIt_UEFI/cpp_lib/XmlLiteParser.cpp | 2 +- rEFIt_UEFI/cpp_lib/XmlLiteParser.h | 11 +- rEFIt_UEFI/cpp_lib/XmlLiteSimpleTypes.cpp | 2 +- 7 files changed, 157 insertions(+), 61 deletions(-) diff --git a/Xcode/CloverConfigPlistValidator/CloverConfigPlistValidator.xcodeproj/xcshareddata/xcschemes/CloverConfigPlistValidator_Debug.xcscheme b/Xcode/CloverConfigPlistValidator/CloverConfigPlistValidator.xcodeproj/xcshareddata/xcschemes/CloverConfigPlistValidator_Debug.xcscheme index e3de2fa4f..353a1a3ed 100644 --- a/Xcode/CloverConfigPlistValidator/CloverConfigPlistValidator.xcodeproj/xcshareddata/xcschemes/CloverConfigPlistValidator_Debug.xcscheme +++ b/Xcode/CloverConfigPlistValidator/CloverConfigPlistValidator.xcodeproj/xcshareddata/xcschemes/CloverConfigPlistValidator_Debug.xcscheme @@ -64,16 +64,24 @@ + + + + + argument = "config-nowarning-noerror.plist" + isEnabled = "NO"> diff --git a/Xcode/CloverConfigPlistValidator/src/main.cpp b/Xcode/CloverConfigPlistValidator/src/main.cpp index c5c53ff1a..eb43b68ad 100755 --- a/Xcode/CloverConfigPlistValidator/src/main.cpp +++ b/Xcode/CloverConfigPlistValidator/src/main.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include "../../../rEFIt_UEFI/Platform/CloverVersion.h" @@ -45,15 +46,77 @@ ssize_t read_all(int fd, void* buf, size_t size) #include "../../../rEFIt_UEFI/Settings/ConfigPlist/ConfigPlistClass.h" -extern "C" int main(int argc, const char * argv[]) +void usage() +{ + fprintf(stderr, "Usage ConfigPlistValidator [-h|--help] [-v|--version] [--info] [-p|--productname=] path_to_config.plist\n"); + exit(1); +} + +extern "C" int main(int argc, char * const argv[]) { (void)argc; (void)argv; setlocale(LC_ALL, "en_US"); // to allow printf unicode char -//AsciiStrHexToUint64("dwf"); + int c; + int info_flag = 0; + XString8 ProductName; - const char* path = NULL; + while (1) + { + static struct option long_options[] = + { + /* These options set a flag. */ + {"info", no_argument, &info_flag, 1}, + {"productname", required_argument, 0, 'p'}, + {"help", no_argument, 0, 'h'}, + {"version", no_argument, 0, 'v'}, + {0, 0, 0, 0} + }; + /* getopt_long stores the option index here. */ + int option_index = 0; + + c = getopt_long (argc, argv, "productname:info:hv", long_options, &option_index); + + /* Detect the end of the options. */ + if (c == -1) + break; + + switch (c) + { + case 0: + break; + + case 'p': + ProductName.takeValueFrom(optarg); + break; + + case 'h': + usage(); + + case 'v': + fprintf(stderr, "ConfigPlistValidator for '%s'\n", gRevisionStr); + fprintf(stderr, "Build id is '%s'\n", gBuildId.c_str()); + break; + + case '?': + /* getopt_long already printed an error message. */ + break; + + default: + fprintf(stderr, "Bug in argument parsing.\n"); + return 2; + } + } + + if (optind == argc) return 1; + if (optind < argc-1 ) { + usage(); + } + + + const char* path = argv[optind]; + #ifdef JIEF_DEBUG path = "config-nowarning-noerror.plist"; path = "config-test2.plist"; @@ -62,16 +125,7 @@ extern "C" int main(int argc, const char * argv[]) //path = "/Volumes/CL_EFI_VMDK/EFI/CLOVER/smbios.plist"; #endif - if ( !path ) { - if ( argc == 2 ) { - path = argv[1]; - }else{ - fprintf(stderr, "ConfigPlistValidator for '%s'\n", gRevisionStr); - fprintf(stderr, "Build id is '%s'\n", gBuildId.c_str()); - fprintf(stderr, "Usage ConfigPlistValidator path_to_config.plist\n"); - return -1; - } - } + struct stat st; int ret = stat(path, &st); if ( ret != 0 ) { @@ -98,20 +152,36 @@ extern "C" int main(int argc, const char * argv[]) XmlLiteParser xmlLiteParser; xmlLiteParser.init(buf, st.st_size); + if ( ProductName.notEmpty() ) { + // if a ProductName is specified in plist, this will be ignored. + configPlistTest.SMBIOS.defaultMacModel = GetModelFromString(ProductName); + } + configPlistTest.parse(&xmlLiteParser, LString8("")); + + if ( ProductName.notEmpty() && configPlistTest.getSMBIOS().getProductName().isDefined() ) { + if ( ProductName != configPlistTest.getSMBIOS().getProductName().value() ) { + printf("Warning: ProductName is specified in command line AND in plist, to a different value. Command line option ignored.\n"); + } + } + bool b = true; for ( size_t idx = 0 ; idx < xmlLiteParser.getXmlParserMessageArray().size() ; idx++ ) { const XmlParserMessage& xmlMsg = xmlLiteParser.getXmlParserMessageArray()[idx]; if ( xmlMsg.type != XmlParserMessageType::info ) { printf("%s\n", xmlMsg.getFormattedMsg().c_str()); b = false; - }else{ + }else + if ( info_flag ) { printf("%s\n", xmlMsg.getFormattedMsg().c_str()); - // One day, create a command line option to display info messages. } } if ( b ) { - printf("Your plist looks so wonderful. Well done!\n"); + if ( xmlLiteParser.getXmlParserInfoMessageCount() > 0 ) { + printf("Your plist looks good. Well done!\n"); + }else{ + printf("Your plist looks so wonderful. Well done!\n"); + } return 0; }else{ return 1; diff --git a/rEFIt_UEFI/Settings/ConfigManager.cpp b/rEFIt_UEFI/Settings/ConfigManager.cpp index 3c1d21bf4..e0d27698e 100644 --- a/rEFIt_UEFI/Settings/ConfigManager.cpp +++ b/rEFIt_UEFI/Settings/ConfigManager.cpp @@ -537,7 +537,15 @@ EFI_STATUS LoadPlist(const XStringW& ConfName, C* plist) DebugLog(2, "%s\n", xmlMsg.getFormattedMsg().c_str()); } } - DebugLog(2, "Use CloverConfigPlistValidator or look in the log\n"); + DebugLog(2, "Use CloverConfigPlistValidator"); + if ( plist->getSMBIOS().dgetModel() < MaxMacModel ) { + if ( xmlLiteParser.productNameNeeded ) DebugLog(2, " (with --productname=%s)", MachineModelName[plist->getSMBIOS().dgetModel()].c_str()); + }else{ + // This is NOT supposed to happen, since CLover set a default mac model + // If a default mac model is not set, a crash would probably happen earlier, but who knows + if ( xmlLiteParser.productNameNeeded ) DebugLog(2, "(with --productname=?)"); + } + DebugLog(2, " or look in the log\n"); } if ( !parsingOk ) { DebugLog(2, "Parsing error while parsing '%ls'.\n", configPlistPath.wc_str()); @@ -905,6 +913,8 @@ EFI_STATUS ConfigManager::LoadConfig(const XStringW& ConfName) { DbgHeader("GetUserSettings"); + DBG("GetDefaultModel()=%s\n", MachineModelName[GetDefaultModel()].c_str()); // GetDefaultModel do NOT return MaxMacModel, so MachineModelName[GetDefaultModel()] is always valid + if ( !selfOem.isInitialized() ) { log_technical_bug("%s : !selfOem.isInitialized()", __PRETTY_FUNCTION__); } @@ -916,19 +926,18 @@ EFI_STATUS ConfigManager::LoadConfig(const XStringW& ConfName) /*Status = */ LoadSMBIOSPlist(L"smbios"_XSW); // we don't need Status. If not loaded correctly, smbiosPlist is !defined and will be ignored by AssignOldNewSettings() - GlobalConfig.CurrentModel = iMac132; - if ( smbiosPlist.SMBIOS.isDefined() && smbiosPlist.SMBIOS.hasModel()) { - GlobalConfig.CurrentModel = smbiosPlist.SMBIOS.getModel(); - } else if ( configPlist.getSMBIOS().hasModel() ) { - GlobalConfig.CurrentModel = configPlist.getSMBIOS().getModel(); + if ( smbiosPlist.getSMBIOS().isDefined() && smbiosPlist.getSMBIOS().getProductName().isDefined() ) { + GlobalConfig.CurrentModel = smbiosPlist.SMBIOS.dgetModel(); + } else if ( configPlist.getSMBIOS().isDefined() && configPlist.getSMBIOS().getProductName().isDefined() ) { + GlobalConfig.CurrentModel = configPlist.getSMBIOS().dgetModel(); } else { - log_technical_bug("No MacModel. SmbiosDictClass::defaultMacModel must be initialized before reading config or smbios plist."); GlobalConfig.CurrentModel = GetDefaultModel(); } if ( !EFI_ERROR(Status) ) { gSettings.takeValueFrom(configPlist); // if load failed, keep default value. } + // Fill in default for model SetDMISettingsForModel(GlobalConfig.CurrentModel, &gSettings); diff --git a/rEFIt_UEFI/Settings/ConfigPlist/SMBIOSPlist.h b/rEFIt_UEFI/Settings/ConfigPlist/SMBIOSPlist.h index bac68f769..735500107 100755 --- a/rEFIt_UEFI/Settings/ConfigPlist/SMBIOSPlist.h +++ b/rEFIt_UEFI/Settings/ConfigPlist/SMBIOSPlist.h @@ -500,47 +500,51 @@ public: bool b = super::validate(xmlLiteParser, xmlPath, keyPos, generateErrors); if ( !ProductName.isDefined() ) { // return xmlLiteParser->addWarning(generateErrors, S8Printf("ProductName is not defined, the whole SMBIOS dict is ignored at line %d.", keyPos.getLine())); - if ( defaultMacModel < MaxMacModel ) { - ProductName.setStringValue(MachineModelName[defaultMacModel]); - } +// if ( defaultMacModel < MaxMacModel ) { +// ProductName.setStringValue(MachineModelName[defaultMacModel]); +// } } - if ( hasModel() ) { + if ( dgetModel() < MaxMacModel ) { if ( BiosVersion.isDefined() ) { - if ( !is2ndBiosVersionGreaterThan1st(ApplePlatformDataArray[getModel()].firmwareVersion, BiosVersion.value()) ) { - xmlLiteParser->addWarning(generateErrors, S8Printf("BiosVersion '%s' is before than default ('%s') -> ignored. Dict '%s:%d'.", BiosVersion.value().c_str(), ApplePlatformDataArray[getModel()].firmwareVersion.c_str(), xmlPath.c_str(), keyPos.getLine())); // Do not set b to false : we don't want to invalidate the whole dict + if ( !is2ndBiosVersionGreaterThan1st(ApplePlatformDataArray[dgetModel()].firmwareVersion, BiosVersion.value()) ) { + xmlLiteParser->addWarning(generateErrors, S8Printf("BiosVersion '%s' is before than default ('%s') -> ignored. Dict '%s:%d'.", BiosVersion.value().c_str(), ApplePlatformDataArray[dgetModel()].firmwareVersion.c_str(), xmlPath.c_str(), keyPos.getLine())); // Do not set b to false : we don't want to invalidate the whole dict + xmlLiteParser->productNameNeeded = !getProductName().isDefined(); BiosVersion.reset(); }else - if ( is2ndBiosVersionEqual(ApplePlatformDataArray[getModel()].firmwareVersion, BiosVersion.value()) ) { + if ( is2ndBiosVersionEqual(ApplePlatformDataArray[dgetModel()].firmwareVersion, BiosVersion.value()) ) { xmlLiteParser->addInfo(generateErrors, S8Printf("BiosVersion '%s' is the same as default. Dict '%s:%d'.", BiosVersion.value().c_str(), xmlPath.c_str(), keyPos.getLine())); // Do not set b to false : we don't want to invalidate the whole dict + xmlLiteParser->productNameNeeded = !getProductName().isDefined(); BiosVersion.reset(); } } if ( BiosReleaseDate.isDefined() ) { - int compareReleaseDateResult = compareReleaseDate(GetReleaseDate(getModel()), BiosReleaseDate.value()); + int compareReleaseDateResult = compareReleaseDate(GetReleaseDate(dgetModel()), BiosReleaseDate.value()); if ( compareReleaseDateResult == 1 ) { - xmlLiteParser->addWarning(generateErrors, S8Printf("BiosReleaseDate '%s' is older than default ('%s') -> ignored. Dict '%s:%d'.", BiosReleaseDate.value().c_str(), GetReleaseDate(getModel()).c_str(), xmlPath.c_str(), keyPos.getLine())); // Do not set b to false : we don't want to invalidate the whole dict + xmlLiteParser->addWarning(generateErrors, S8Printf("BiosReleaseDate '%s' is older than default ('%s') -> ignored. Dict '%s:%d'.", BiosReleaseDate.value().c_str(), GetReleaseDate(dgetModel()).c_str(), xmlPath.c_str(), keyPos.getLine())); // Do not set b to false : we don't want to invalidate the whole dict + xmlLiteParser->productNameNeeded = !getProductName().isDefined(); BiosReleaseDate.reset(); }else if ( compareReleaseDateResult == 0 ) { // This is just 'info'. It's useless but fine to define the same as default. xmlLiteParser->addInfo(generateErrors, S8Printf("BiosReleaseDate '%s' is the same as default. Dict '%s:%d'.", BiosReleaseDate.value().c_str(), xmlPath.c_str(), keyPos.getLine())); // Do not set b to false : we don't want to invalidate the whole dict + xmlLiteParser->productNameNeeded = !getProductName().isDefined(); BiosReleaseDate.reset(); } } if ( EfiVersion.isDefined() ) { if ( AsciiStrVersionToUint64(ApplePlatformDataArray[dgetModel()].efiversion, 4, 5) > AsciiStrVersionToUint64(EfiVersion.value(), 4, 5)) { xmlLiteParser->addWarning(generateErrors, S8Printf("EfiVersion '%s' is older than default ('%s') -> ignored. Dict '%s:%d'.", EfiVersion.value().c_str(), ApplePlatformDataArray[dgetModel()].efiversion.c_str(), xmlPath.c_str(), keyPos.getLine())); // Do not set b to false : we don't want to invalidate the whole dict + xmlLiteParser->productNameNeeded = !getProductName().isDefined(); EfiVersion.reset(); } else if (AsciiStrVersionToUint64(ApplePlatformDataArray[dgetModel()].efiversion, 4, 5) == AsciiStrVersionToUint64(EfiVersion.value(), 4, 5)) { xmlLiteParser->addInfo(generateErrors, S8Printf("EfiVersion '%s' is the same as default. Dict '%s:%d'.", EfiVersion.value().c_str(), xmlPath.c_str(), keyPos.getLine())); // Do not set b to false : we don't want to invalidate the whole dict + xmlLiteParser->productNameNeeded = !getProductName().isDefined(); EfiVersion.reset(); } } }else{ // This is supposed to never happen within Clover, because Clover initialise defaultMacModel. - // ccpv doesn't initialise defaultMacModel yet. - // If ccpv, let's say nothing at the moment - //xmlLiteParser->addInfo(generateErrors, S8Printf("Cannot check validity of BiosVersion because ProductName is not set. Dict '%s:%d'.", xmlPath.c_str(), keyPos.getLine())); + xmlLiteParser->addInfo(generateErrors, S8Printf("Cannot check validity of BiosVersion, BiosReleaseDate and EfiVersion because ProductName is not set in dict '%s:%d'. Define ProductName or run this tool with --productname=[your product name].", xmlPath.c_str(), keyPos.getLine())); } return b; } @@ -580,25 +584,26 @@ public: const decltype(NoRomInfo)& getNoRomInfo() const { return NoRomInfo; } - /* - * DO NOT call this if !ProductName.isDefined() - */ - MacModel getModel() const - { - if ( !ProductName.isDefined() ) { - // This must not happen in Clover because Clover set a defaultMacModel - // This must ot happen in ccpv because ccpv doesn't call dget... methods - log_technical_bug("%s : !ProductName.isDefined()", __PRETTY_FUNCTION__); - return iMac132; // cannot return GetDefaultModel() because we don't want to link runtime configuration to the xml reading layer. - } - return GetModelFromString(ProductName.value()); // ProductName has been validated, so Model CANNOT be MaxMacModel - } - XBool hasModel() const { return ProductName.isDefined(); } +// /* +// * DO NOT call this if !ProductName.isDefined() +// */ +// MacModel getModel() const +// { +// if ( !ProductName.isDefined() ) { +// // This must not happen in Clover because Clover set a defaultMacModel +// // This must ot happen in ccpv because ccpv doesn't call dget... methods +// log_technical_bug("%s : !ProductName.isDefined()", __PRETTY_FUNCTION__); +// return iMac132; // cannot return GetDefaultModel() because we don't want to link runtime configuration to the xml reading layer. +// } +// return GetModelFromString(ProductName.value()); // ProductName has been validated, so Model CANNOT be MaxMacModel +// } +// XBool hasModel() const { return ProductName.isDefined(); } MacModel dgetModel() const { - if ( !hasModel() ) return MaxMacModel; - return getModel(); + if ( ProductName.isDefined() ) return GetModelFromString(ProductName.value()); + if ( defaultMacModel < MaxMacModel ) return defaultMacModel; + return MaxMacModel; } decltype(BiosVendor)::ValueType dgetBiosVendor() const { @@ -764,6 +769,9 @@ public: public: SmbiosPlistClass() {}; + const decltype(SMBIOS)& getSMBIOS() const { return SMBIOS; }; + + }; diff --git a/rEFIt_UEFI/cpp_lib/XmlLiteParser.cpp b/rEFIt_UEFI/cpp_lib/XmlLiteParser.cpp index 51a619c0c..11904d8f0 100755 --- a/rEFIt_UEFI/cpp_lib/XmlLiteParser.cpp +++ b/rEFIt_UEFI/cpp_lib/XmlLiteParser.cpp @@ -65,7 +65,7 @@ void XmlLiteParser::init(const char* buf, size_t size) currentPos.line = 1; currentPos.col = 1; - errorsAndWarnings.setEmpty(); + XmlParserMessageArray.setEmpty(); for ( size_t i = 0; i < size ; ++i) { if ( p_start[i] == 0 ) { diff --git a/rEFIt_UEFI/cpp_lib/XmlLiteParser.h b/rEFIt_UEFI/cpp_lib/XmlLiteParser.h index 2797cc2e2..8f01d34d5 100755 --- a/rEFIt_UEFI/cpp_lib/XmlLiteParser.h +++ b/rEFIt_UEFI/cpp_lib/XmlLiteParser.h @@ -76,15 +76,16 @@ class XmlLiteParser char* p_start = NULL; char* p_end = NULL; XmlParserPosition currentPos = XmlParserPosition(); - XObjArray errorsAndWarnings = XObjArray(); + XObjArray XmlParserMessageArray = XObjArray(); XBool AddXmlParserMessage(XmlParserMessage* msg) { - if ( errorsAndWarnings.size() < 500 ) errorsAndWarnings.AddReference(msg, true); - if ( errorsAndWarnings.size() == 500 ) errorsAndWarnings.AddReference(new XmlParserMessage(XmlParserMessageType::error, "Too many error. Stopping"_XS8), true); + if ( XmlParserMessageArray.size() < 500 ) XmlParserMessageArray.AddReference(msg, true); + if ( XmlParserMessageArray.size() == 500 ) XmlParserMessageArray.AddReference(new XmlParserMessage(XmlParserMessageType::error, "Too many error. Stopping"_XS8), true); return false; } public: XBool xmlParsingError = false; + XBool productNameNeeded = false; XmlLiteParser() {}; XmlLiteParser(char* _p) : p_start(_p) @@ -99,8 +100,8 @@ public: int getLine() { return currentPos.line; } int getCol() { return currentPos.col; } - XObjArray& getXmlParserMessageArray() { return errorsAndWarnings; } - size_t getXmlParserInfoMessageCount() const { size_t n=0 ; for ( size_t i=0 ; i < errorsAndWarnings.size() ; i++ ) if ( errorsAndWarnings[i].type == XmlParserMessageType::info ) ++n; return n; } + XObjArray& getXmlParserMessageArray() { return XmlParserMessageArray; } + size_t getXmlParserInfoMessageCount() const { size_t n=0 ; for ( size_t i=0 ; i < XmlParserMessageArray.size() ; i++ ) if ( XmlParserMessageArray[i].type == XmlParserMessageType::info ) ++n; return n; } // Add warning, error and xml error always return false so you can return addWarning(...) from validate function XBool addInfo(XBool generateErrors, const XString8& warning) { if ( generateErrors ) AddXmlParserMessage(new XmlParserMessage(XmlParserMessageType::info, warning)); return false; } XBool addWarning(XBool generateErrors, const XString8& warning) { if ( generateErrors ) AddXmlParserMessage(new XmlParserMessage(XmlParserMessageType::warning, warning)); return false; } diff --git a/rEFIt_UEFI/cpp_lib/XmlLiteSimpleTypes.cpp b/rEFIt_UEFI/cpp_lib/XmlLiteSimpleTypes.cpp index 22491425b..842338ec4 100644 --- a/rEFIt_UEFI/cpp_lib/XmlLiteSimpleTypes.cpp +++ b/rEFIt_UEFI/cpp_lib/XmlLiteSimpleTypes.cpp @@ -379,7 +379,7 @@ XBool XmlIntegerAbstract::parseXmlInteger(XmlLiteParser* xmlLiteParser, const XS XBool atLeastOneDigit = false; #ifdef JIEF_DEBUG -if ( xmlPath.contains("CsrActiveConfig") ) { +if ( xmlPath.contains("Timeout") ) { int i=0; (void)i; } #endif