Skip to content

Commit ea70035

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 ea70035

40 files changed

+97
-99
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/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;

Kernel/Bus/USB/USBManagement.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ UNMAP_AFTER_INIT USBManagement::USBManagement()
2323
void USBManagement::register_driver(NonnullLockRefPtr<Driver> driver)
2424
{
2525
dbgln_if(USB_DEBUG, "Registering driver {}", driver->name());
26-
s_available_drivers->append(driver);
26+
s_available_drivers->try_append(driver).release_value_but_fixme_should_propagate_errors();
2727
}
2828

2929
LockRefPtr<Driver> USBManagement::get_driver_by_name(StringView name)

Kernel/Bus/VirtIO/Transport/PCIe/TransportLink.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ ErrorOr<void> PCIeTransportLink::locate_configurations_and_resources(Badge<VirtI
131131
else if (config.cfg_type == ConfigurationType::Notify)
132132
m_notify_multiplier = capability.read32(0x10);
133133

134-
m_configs.append(config);
134+
TRY(m_configs.try_append(config));
135135
}
136136
}
137137

0 commit comments

Comments
 (0)