Skip to content

Commit 31269f4

Browse files
committed
AK+Kernel+Userland: Don't allow Vector::append() in the kernel
This makes it not as easy to forgot to handle OOMs in the kernel. This commit replaces most usages of this function with `try_append(...).release_value_but_fixme_should_propagate_errors()`. But in some cases, using the `TRY` macro or `unchecked_append()` is already possible. In places where allocations should not fail or an OOM would be fatal anyways, `MUST(try_append(...))` should be used explicitly.
1 parent 642866d commit 31269f4

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+103
-105
lines changed

AK/StringView.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ Vector<StringView> StringView::split_view(StringView separator, SplitBehavior sp
6363
{
6464
Vector<StringView> parts;
6565
for_each_split_view(separator, split_behavior, [&](StringView view) {
66-
parts.append(view);
66+
parts.try_append(view).release_value_but_fixme_should_propagate_errors();
6767
});
6868
return parts;
6969
}

AK/Vector.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -271,8 +271,6 @@ requires(!IsRvalueReference<T>) class Vector {
271271
MUST(try_extend(other));
272272
}
273273

274-
#endif
275-
276274
ALWAYS_INLINE void append(T&& value)
277275
{
278276
if constexpr (contains_reference)
@@ -287,7 +285,6 @@ requires(!IsRvalueReference<T>) class Vector {
287285
MUST(try_append(T(value)));
288286
}
289287

290-
#ifndef KERNEL
291288
void append(StorageType const* values, size_t count)
292289
{
293290
MUST(try_append(values, count));

Kernel/Arch/aarch64/InterruptManagement.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ void InterruptManagement::initialize()
3636

3737
void InterruptManagement::add_recipe(DeviceTree::DeviceRecipe<NonnullLockRefPtr<IRQController>> recipe)
3838
{
39-
s_recipes->append(move(recipe));
39+
MUST(s_recipes->try_append(move(recipe)));
4040
}
4141

4242
void InterruptManagement::find_controllers()
@@ -48,7 +48,7 @@ void InterruptManagement::find_controllers()
4848
continue;
4949
}
5050

51-
m_interrupt_controllers.append(device_or_error.release_value());
51+
MUST(m_interrupt_controllers.try_append(device_or_error.release_value()));
5252
}
5353

5454
if (m_interrupt_controllers.is_empty())

Kernel/Arch/riscv64/InterruptManagement.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ void InterruptManagement::initialize()
3838

3939
void InterruptManagement::add_recipe(DeviceTree::DeviceRecipe<NonnullLockRefPtr<IRQController>> recipe)
4040
{
41-
s_recipes->append(move(recipe));
41+
MUST(s_recipes->try_append(move(recipe)));
4242
}
4343

4444
void InterruptManagement::find_controllers()
@@ -50,7 +50,7 @@ void InterruptManagement::find_controllers()
5050
continue;
5151
}
5252

53-
m_interrupt_controllers.append(device_or_error.release_value());
53+
MUST(m_interrupt_controllers.try_append(device_or_error.release_value()));
5454
}
5555

5656
if (m_interrupt_controllers.is_empty())

Kernel/Arch/x86_64/Firmware/ACPI.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ Optional<PhysicalAddress> find_rsdp_in_ia_pc_specific_memory_locations()
3333
if (!(memory_range.type == Memory::PhysicalMemoryRangeType::ACPI_NVS || memory_range.type == Memory::PhysicalMemoryRangeType::ACPI_Reclaimable))
3434
return IterationDecision::Continue;
3535

36-
potential_ranges.append(memory_range);
36+
potential_ranges.try_append(memory_range).release_value_but_fixme_should_propagate_errors();
3737
return IterationDecision::Continue;
3838
});
3939

Kernel/Arch/x86_64/InterruptManagement.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ UNMAP_AFTER_INIT void InterruptManagement::switch_to_pic_mode()
137137
VERIFY(m_interrupt_controllers.is_empty());
138138
dmesgln("Interrupts: Switch to Legacy PIC mode");
139139
InterruptDisabler disabler;
140-
m_interrupt_controllers.append(adopt_lock_ref(*new PIC()));
140+
m_interrupt_controllers.try_append(adopt_lock_ref(*new PIC())).release_value_but_fixme_should_propagate_errors();
141141
SpuriousInterruptHandler::initialize(7);
142142
SpuriousInterruptHandler::initialize(15);
143143
dbgln("Interrupts: Detected {}", m_interrupt_controllers[0]->model());
@@ -186,7 +186,7 @@ UNMAP_AFTER_INIT void InterruptManagement::locate_apic_data()
186186
auto madt = Memory::map_typed<ACPI::Structures::MADT>(m_madt_physical_address.value()).release_value_but_fixme_should_propagate_errors();
187187

188188
if (madt->flags & PCAT_COMPAT_FLAG)
189-
m_interrupt_controllers.append(adopt_lock_ref(*new PIC()));
189+
m_interrupt_controllers.try_append(adopt_lock_ref(*new PIC())).release_value_but_fixme_should_propagate_errors();
190190
size_t entry_index = 0;
191191
size_t entries_length = madt->h.length - sizeof(ACPI::Structures::MADT);
192192
auto* madt_entry = madt->entries;
@@ -195,7 +195,7 @@ UNMAP_AFTER_INIT void InterruptManagement::locate_apic_data()
195195
if (madt_entry->type == (u8)ACPI::Structures::MADTEntryType::IOAPIC) {
196196
auto* ioapic_entry = (const ACPI::Structures::MADTEntries::IOAPIC*)madt_entry;
197197
dbgln("IOAPIC found @ MADT entry {}, MMIO Registers @ {}", entry_index, PhysicalAddress(ioapic_entry->ioapic_address));
198-
m_interrupt_controllers.append(adopt_lock_ref(*new IOAPIC(PhysicalAddress(ioapic_entry->ioapic_address), ioapic_entry->gsi_base)));
198+
m_interrupt_controllers.try_append(adopt_lock_ref(*new IOAPIC(PhysicalAddress(ioapic_entry->ioapic_address), ioapic_entry->gsi_base))).release_value_but_fixme_should_propagate_errors();
199199
}
200200
if (madt_entry->type == (u8)ACPI::Structures::MADTEntryType::InterruptSourceOverride) {
201201
auto* interrupt_override_entry = (const ACPI::Structures::MADTEntries::InterruptSourceOverride*)madt_entry;

Kernel/Arch/x86_64/Time/HPET.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,7 @@ Vector<unsigned> HPET::capable_interrupt_numbers(HPETComparator const& comparato
339339
u32 interrupt_bitfield = comparator_registers.interrupt_routing;
340340
for (size_t index = 0; index < 32; index++) {
341341
if (interrupt_bitfield & 1)
342-
capable_interrupts.append(index);
342+
capable_interrupts.try_append(index).release_value_but_fixme_should_propagate_errors();
343343
interrupt_bitfield >>= 1;
344344
}
345345
return capable_interrupts;
@@ -353,7 +353,7 @@ Vector<unsigned> HPET::capable_interrupt_numbers(u8 comparator_number)
353353
u32 interrupt_bitfield = comparator_registers.interrupt_routing;
354354
for (size_t index = 0; index < 32; index++) {
355355
if (interrupt_bitfield & 1)
356-
capable_interrupts.append(index);
356+
capable_interrupts.try_append(index).release_value_but_fixme_should_propagate_errors();
357357
interrupt_bitfield >>= 1;
358358
}
359359
return capable_interrupts;
@@ -454,8 +454,8 @@ UNMAP_AFTER_INIT HPET::HPET(PhysicalAddress acpi_hpet)
454454
if (regs.capabilities.attributes & (u32)HPETFlags::Attributes::LegacyReplacementRouteCapable)
455455
regs.configuration.low = regs.configuration.low | (u32)HPETFlags::Configuration::LegacyReplacementRoute;
456456

457-
m_comparators.append(HPETComparator::create(0, 0, is_periodic_capable(0), is_64bit_capable(0)));
458-
m_comparators.append(HPETComparator::create(1, 8, is_periodic_capable(1), is_64bit_capable(1)));
457+
m_comparators.try_append(HPETComparator::create(0, 0, is_periodic_capable(0), is_64bit_capable(0))).release_value_but_fixme_should_propagate_errors();
458+
m_comparators.try_append(HPETComparator::create(1, 8, is_periodic_capable(1), is_64bit_capable(1))).release_value_but_fixme_should_propagate_errors();
459459

460460
global_enable();
461461
}

Kernel/Boot/CommandLine.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ Vector<NonnullOwnPtr<KString>> CommandLine::userspace_init_args() const
315315
if (!init_args.is_empty())
316316
MUST(args.try_prepend(MUST(KString::try_create(userspace_init()))));
317317
for (auto& init_arg : init_args)
318-
args.append(MUST(KString::try_create(init_arg)));
318+
MUST(args.try_append(MUST(KString::try_create(init_arg))));
319319
return args;
320320
}
321321

Kernel/Bus/PCI/Access.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ ErrorOr<void> Access::add_host_controller_and_scan_for_devices(NonnullOwnPtr<Hos
176176
dmesgln("Failed during PCI Access::rescan_hardware due to {}", device_identifier_or_error.error());
177177
VERIFY_NOT_REACHED();
178178
}
179-
m_device_identifiers.append(device_identifier_or_error.release_value());
179+
m_device_identifiers.try_append(device_identifier_or_error.release_value()).release_value_but_fixme_should_propagate_errors();
180180
});
181181
return {};
182182
}
@@ -208,7 +208,7 @@ UNMAP_AFTER_INIT void Access::rescan_hardware()
208208
dmesgln("Failed during PCI Access::rescan_hardware due to {}", device_identifier_or_error.error());
209209
VERIFY_NOT_REACHED();
210210
}
211-
m_device_identifiers.append(device_identifier_or_error.release_value());
211+
m_device_identifiers.try_append(device_identifier_or_error.release_value()).release_value_but_fixme_should_propagate_errors();
212212
});
213213
}
214214
}

Kernel/Bus/PCI/Controller/HostController.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ UNMAP_AFTER_INIT Vector<Capability> HostController::get_capabilities_for_functio
9999
u8 capability_id = capability_header & 0xff;
100100

101101
// FIXME: Don't attach a PCI address to a capability object
102-
capabilities.append({ Address(domain_number(), bus.value(), device.value(), function.value()), capability_id, capability_pointer });
102+
capabilities.try_append({ Address(domain_number(), bus.value(), device.value(), function.value()), capability_id, capability_pointer }).release_value_but_fixme_should_propagate_errors();
103103
capability_pointer = capability_header >> 8;
104104
}
105105
return capabilities;

0 commit comments

Comments
 (0)