From b790cfcef127cc619024630aa9d74b23eece3899 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herm=C3=A8s=20B=C3=A9lusca-Ma=C3=AFto?= Date: Sat, 9 Mar 2024 11:45:27 +0100 Subject: [PATCH] [FREELDR] Pre-initialize the INI section list, improve loops over sections and items. Avoids dereferencing list entries to sections/items when these lists are empty. IniParseFile(): Emit an error to the debug log when a candidate setting is outside a section and skip it, instead of popping up an error on the UI. --- boot/freeldr/freeldr/lib/inifile/inifile.c | 82 ++++++++++++---------- boot/freeldr/freeldr/lib/inifile/parse.c | 14 ++-- 2 files changed, 50 insertions(+), 46 deletions(-) diff --git a/boot/freeldr/freeldr/lib/inifile/inifile.c b/boot/freeldr/freeldr/lib/inifile/inifile.c index a15102f28da..a1bd7c9f91a 100644 --- a/boot/freeldr/freeldr/lib/inifile/inifile.c +++ b/boot/freeldr/freeldr/lib/inifile/inifile.c @@ -24,14 +24,18 @@ DBG_DEFAULT_CHANNEL(INIFILE); BOOLEAN IniOpenSection(PCSTR SectionName, ULONG_PTR* SectionId) { - PINI_SECTION Section; + PLIST_ENTRY Entry; + PINI_SECTION Section; TRACE("IniOpenSection() SectionName = %s\n", SectionName); - // Loop through each section and find the one they want - Section = CONTAINING_RECORD(IniFileSectionListHead.Flink, INI_SECTION, ListEntry); - while (&Section->ListEntry != &IniFileSectionListHead) + // Loop through each section and find the one we want + for (Entry = IniFileSectionListHead.Flink; + Entry != &IniFileSectionListHead; + Entry = Entry->Flink) { + Section = CONTAINING_RECORD(Entry, INI_SECTION, ListEntry); + // Compare against the section name if (_stricmp(SectionName, Section->SectionName) == 0) { @@ -41,9 +45,6 @@ BOOLEAN IniOpenSection(PCSTR SectionName, ULONG_PTR* SectionId) TRACE("IniOpenSection() Found it! SectionId = 0x%x\n", SectionId); return TRUE; } - - // Get the next section in the list - Section = CONTAINING_RECORD(Section->ListEntry.Flink, INI_SECTION, ListEntry); } TRACE("IniOpenSection() Section not found.\n"); @@ -53,7 +54,7 @@ BOOLEAN IniOpenSection(PCSTR SectionName, ULONG_PTR* SectionId) ULONG IniGetNumSectionItems(ULONG_PTR SectionId) { - PINI_SECTION Section = (PINI_SECTION)SectionId; + PINI_SECTION Section = (PINI_SECTION)SectionId; TRACE("IniGetNumSectionItems() SectionId = 0x%x\n", SectionId); TRACE("IniGetNumSectionItems() Item count = %d\n", Section->SectionItemCount); @@ -63,14 +64,18 @@ ULONG IniGetNumSectionItems(ULONG_PTR SectionId) PINI_SECTION_ITEM IniGetSettingByNumber(ULONG_PTR SectionId, ULONG SettingNumber) { - PINI_SECTION Section = (PINI_SECTION)SectionId; - PINI_SECTION_ITEM SectionItem; + PINI_SECTION Section = (PINI_SECTION)SectionId; + PLIST_ENTRY Entry; + PINI_SECTION_ITEM SectionItem; - // Loop through each section item and find the one they want - SectionItem = CONTAINING_RECORD(Section->SectionItemList.Flink, INI_SECTION_ITEM, ListEntry); - while (&SectionItem->ListEntry != &Section->SectionItemList) + // Loop through each section item and find the one we want + for (Entry = Section->SectionItemList.Flink; + Entry != &Section->SectionItemList; + Entry = Entry->Flink) { - // Check to see if this is the setting they want + SectionItem = CONTAINING_RECORD(Entry, INI_SECTION_ITEM, ListEntry); + + // Check to see if this is the setting we want if (SettingNumber == 0) { return SectionItem; @@ -78,16 +83,13 @@ PINI_SECTION_ITEM IniGetSettingByNumber(ULONG_PTR SectionId, ULONG SettingNumber // Nope, keep going SettingNumber--; - - // Get the next section item in the list - SectionItem = CONTAINING_RECORD(SectionItem->ListEntry.Flink, INI_SECTION_ITEM, ListEntry); } return NULL; } ULONG IniGetSectionSettingNameSize(ULONG_PTR SectionId, ULONG SettingIndex) { - PINI_SECTION_ITEM SectionItem; + PINI_SECTION_ITEM SectionItem; // Retrieve requested setting SectionItem = IniGetSettingByNumber(SectionId, SettingIndex); @@ -100,7 +102,7 @@ ULONG IniGetSectionSettingNameSize(ULONG_PTR SectionId, ULONG SettingIndex) ULONG IniGetSectionSettingValueSize(ULONG_PTR SectionId, ULONG SettingIndex) { - PINI_SECTION_ITEM SectionItem; + PINI_SECTION_ITEM SectionItem; // Retrieve requested setting SectionItem = IniGetSettingByNumber(SectionId, SettingIndex); @@ -146,16 +148,20 @@ BOOLEAN IniReadSettingByNumber(ULONG_PTR SectionId, ULONG SettingNumber, PCHAR S BOOLEAN IniReadSettingByName(ULONG_PTR SectionId, PCSTR SettingName, PCHAR Buffer, ULONG BufferSize) { - PINI_SECTION Section = (PINI_SECTION)SectionId; - PINI_SECTION_ITEM SectionItem; + PINI_SECTION Section = (PINI_SECTION)SectionId; + PLIST_ENTRY Entry; + PINI_SECTION_ITEM SectionItem; TRACE("IniReadSettingByName() SectionId = 0x%x\n", SectionId); - // Loop through each section item and find the one they want - SectionItem = CONTAINING_RECORD(Section->SectionItemList.Flink, INI_SECTION_ITEM, ListEntry); - while (&SectionItem->ListEntry != &Section->SectionItemList) + // Loop through each section item and find the one we want + for (Entry = Section->SectionItemList.Flink; + Entry != &Section->SectionItemList; + Entry = Entry->Flink) { - // Check to see if this is the setting they want + SectionItem = CONTAINING_RECORD(Entry, INI_SECTION_ITEM, ListEntry); + + // Check to see if this is the setting we want if (_stricmp(SettingName, SectionItem->ItemName) == 0) { TRACE("IniReadSettingByName() Setting \'%s\' found.\n", SettingName); @@ -166,9 +172,6 @@ BOOLEAN IniReadSettingByName(ULONG_PTR SectionId, PCSTR SettingName, PCHAR Buffe return TRUE; } - - // Get the next section item in the list - SectionItem = CONTAINING_RECORD(SectionItem->ListEntry.Flink, INI_SECTION_ITEM, ListEntry); } WARN("IniReadSettingByName() Setting \'%s\' not found.\n", SettingName); @@ -178,7 +181,7 @@ BOOLEAN IniReadSettingByName(ULONG_PTR SectionId, PCSTR SettingName, PCHAR Buffe BOOLEAN IniAddSection(PCSTR SectionName, ULONG_PTR* SectionId) { - PINI_SECTION Section; + PINI_SECTION Section; // Allocate a new section structure Section = FrLdrTempAlloc(sizeof(INI_SECTION), TAG_INI_SECTION); @@ -251,8 +254,8 @@ VOID IniCleanup(VOID) BOOLEAN IniAddSettingValueToSection(ULONG_PTR SectionId, PCSTR SettingName, PCSTR SettingValue) { - PINI_SECTION Section = (PINI_SECTION)SectionId; - PINI_SECTION_ITEM SectionItem; + PINI_SECTION Section = (PINI_SECTION)SectionId; + PINI_SECTION_ITEM SectionItem; // Allocate a new item structure SectionItem = FrLdrTempAlloc(sizeof(INI_SECTION_ITEM), TAG_INI_SECTION_ITEM); @@ -292,26 +295,27 @@ BOOLEAN IniAddSettingValueToSection(ULONG_PTR SectionId, PCSTR SettingName, PCST BOOLEAN IniModifySettingValue(ULONG_PTR SectionId, PCSTR SettingName, PCSTR SettingValue) { - PINI_SECTION Section = (PINI_SECTION)SectionId; - PINI_SECTION_ITEM SectionItem; + PINI_SECTION Section = (PINI_SECTION)SectionId; + PLIST_ENTRY Entry; + PINI_SECTION_ITEM SectionItem; PCHAR NewItemValue; // Loop through each section item and find the one we want - SectionItem = CONTAINING_RECORD(Section->SectionItemList.Flink, INI_SECTION_ITEM, ListEntry); - while (&SectionItem->ListEntry != &Section->SectionItemList) + for (Entry = Section->SectionItemList.Flink; + Entry != &Section->SectionItemList; + Entry = Entry->Flink) { + SectionItem = CONTAINING_RECORD(Entry, INI_SECTION_ITEM, ListEntry); + // Check to see if this is the setting we want if (_stricmp(SectionItem->ItemName, SettingName) == 0) { break; } - // Nope, keep going - // Get the next section item in the list - SectionItem = CONTAINING_RECORD(SectionItem->ListEntry.Flink, INI_SECTION_ITEM, ListEntry); } // If the section item does not exist, create it - if (&SectionItem->ListEntry == &Section->SectionItemList) + if (Entry == &Section->SectionItemList) { return IniAddSettingValueToSection(SectionId, SettingName, SettingValue); } diff --git a/boot/freeldr/freeldr/lib/inifile/parse.c b/boot/freeldr/freeldr/lib/inifile/parse.c index 9a3c161e0ab..ab36448e40e 100644 --- a/boot/freeldr/freeldr/lib/inifile/parse.c +++ b/boot/freeldr/freeldr/lib/inifile/parse.c @@ -22,10 +22,10 @@ #include DBG_DEFAULT_CHANNEL(INIFILE); -LIST_ENTRY IniFileSectionListHead; -BOOLEAN IniFileSectionInitialized = FALSE; -ULONG IniFileSectionCount = 0; -ULONG IniFileSettingCount = 0; +LIST_ENTRY IniFileSectionListHead = {&IniFileSectionListHead, &IniFileSectionListHead}; +BOOLEAN IniFileSectionInitialized = FALSE; +ULONG IniFileSectionCount = 0; +ULONG IniFileSettingCount = 0; BOOLEAN IniParseFile(PCHAR IniFileData, ULONG IniFileSize) @@ -122,9 +122,9 @@ BOOLEAN IniParseFile(PCHAR IniFileData, ULONG IniFileSize) // First check to make sure we're inside a [section] if (CurrentSection == NULL) { - printf("Error: freeldr.ini:%lu: Setting '%s' found outside of a [section].\n", CurrentLineNumber, IniFileLine); - printf("Press any key to continue...\n"); - MachConsGetCh(); + ERR("Error: freeldr.ini:%lu: Setting '%s' found outside of a [section].\n", CurrentLineNumber, IniFileLine); + + // Skip it CurrentLineNumber++; continue; }