RGBCube/serenity
1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-25 16:37:35 +00:00
Code Issues Activity Actions

Kernel/KCOV: Bring closer to typical SerenityOS coding style

Browse source 
- Remove a bunch of redundant `this->`
- Make class data members private and provide accessors instead
This commit is contained in:
Andreas Kling 2021-09-06 00:31:48 +02:00
parent 79fbad6df9
commit 91fe6b6552
4 changed files with 44 additions and 39 deletions
Download patch file Download diff file Expand all files Collapse all files

31
Kernel/Devices/KCOVDevice.cpp
View file

@ -41,8 +41,8 @@ void KCOVDevice::free_thread()
return; return;
auto kcov_instance = maybe_kcov_instance.value(); auto kcov_instance = maybe_kcov_instance.value();
VERIFY(kcov_instance->state == KCOVInstance::TRACING); VERIFY(kcov_instance->state() == KCOVInstance::TRACING);
kcov_instance->state = KCOVInstance::OPENED; kcov_instance->set_state(KCOVInstance::OPENED);
thread_instance->remove(tid); thread_instance->remove(tid);
} }
@ -55,8 +55,8 @@ void KCOVDevice::free_process()
return; return;
auto kcov_instance = maybe_kcov_instance.value(); auto kcov_instance = maybe_kcov_instance.value();
VERIFY(kcov_instance->state == KCOVInstance::OPENED); VERIFY(kcov_instance->state() == KCOVInstance::OPENED);
kcov_instance->state = KCOVInstance::UNUSED; kcov_instance->set_state(KCOVInstance::UNUSED);
proc_instance->remove(pid); proc_instance->remove(pid);
delete kcov_instance; delete kcov_instance;
} }
@ -67,7 +67,7 @@ KResultOr<NonnullRefPtr<FileDescription>> KCOVDevice::open(int options)
if (proc_instance->get(pid).has_value()) if (proc_instance->get(pid).has_value())
return EBUSY; // This process already open()ed the kcov device return EBUSY; // This process already open()ed the kcov device
auto kcov_instance = new KCOVInstance(pid); auto kcov_instance = new KCOVInstance(pid);
kcov_instance->state = KCOVInstance::OPENED; kcov_instance->set_state(KCOVInstance::OPENED);
proc_instance->set(pid, kcov_instance); proc_instance->set(pid, kcov_instance);
return File::open(options); return File::open(options);
@ -84,10 +84,10 @@ KResult KCOVDevice::ioctl(FileDescription&, unsigned request, Userspace<void*> a
return ENXIO; // This proc hasn't opened the kcov dev yet return ENXIO; // This proc hasn't opened the kcov dev yet
auto kcov_instance = maybe_kcov_instance.value(); auto kcov_instance = maybe_kcov_instance.value();
SpinlockLocker lock(kcov_instance->lock); SpinlockLocker locker(kcov_instance->spinlock());
switch (request) { switch (request) {
case KCOV_SETBUFSIZE: { case KCOV_SETBUFSIZE: {
if (kcov_instance->state >= KCOVInstance::TRACING) { if (kcov_instance->state() >= KCOVInstance::TRACING) {
return_value = EBUSY; return_value = EBUSY;
break; break;
} }
@ -95,7 +95,7 @@ KResult KCOVDevice::ioctl(FileDescription&, unsigned request, Userspace<void*> a
break; break;
} }
case KCOV_ENABLE: { case KCOV_ENABLE: {
if (kcov_instance->state >= KCOVInstance::TRACING) { if (kcov_instance->state() >= KCOVInstance::TRACING) {
return_value = EBUSY; return_value = EBUSY;
break; break;
} }
@ -103,8 +103,8 @@ KResult KCOVDevice::ioctl(FileDescription&, unsigned request, Userspace<void*> a
return_value = ENOBUFS; return_value = ENOBUFS;
break; break;
} }
VERIFY(kcov_instance->state == KCOVInstance::OPENED); VERIFY(kcov_instance->state() == KCOVInstance::OPENED);
kcov_instance->state = KCOVInstance::TRACING; kcov_instance->set_state(KCOVInstance::TRACING);
thread_instance->set(tid, kcov_instance); thread_instance->set(tid, kcov_instance);
break; break;
} }
@ -114,8 +114,8 @@ KResult KCOVDevice::ioctl(FileDescription&, unsigned request, Userspace<void*> a
return_value = ENOENT; return_value = ENOENT;
break; break;
} }
VERIFY(kcov_instance->state == KCOVInstance::TRACING); VERIFY(kcov_instance->state() == KCOVInstance::TRACING);
kcov_instance->state = KCOVInstance::OPENED; kcov_instance->set_state(KCOVInstance::OPENED);
thread_instance->remove(tid); thread_instance->remove(tid);
break; break;
} }
@ -134,12 +134,11 @@ KResultOr<Memory::Region*> KCOVDevice::mmap(Process& process, FileDescription&,
VERIFY(maybe_kcov_instance.has_value()); // Should happen on fd open() VERIFY(maybe_kcov_instance.has_value()); // Should happen on fd open()
auto kcov_instance = maybe_kcov_instance.value(); auto kcov_instance = maybe_kcov_instance.value();
if (!kcov_instance->vmobject) { if (!kcov_instance->vmobject())
return ENOBUFS; // Mmaped, before KCOV_SETBUFSIZE return ENOBUFS; // mmaped, before KCOV_SETBUFSIZE
}
return process.address_space().allocate_region_with_vmobject( return process.address_space().allocate_region_with_vmobject(
range, *kcov_instance->vmobject, offset, {}, prot, shared); range, *kcov_instance->vmobject(), offset, {}, prot, shared);
} }
String KCOVDevice::device_name() const String KCOVDevice::device_name() const

27
Kernel/Devices/KCOVInstance.cpp
View file

@ -12,7 +12,6 @@ namespace Kernel {
KCOVInstance::KCOVInstance(ProcessID pid) KCOVInstance::KCOVInstance(ProcessID pid)
{ {
m_pid = pid; m_pid = pid;
state = UNUSED;
} }
KResult KCOVInstance::buffer_allocate(size_t buffer_size_in_entries) KResult KCOVInstance::buffer_allocate(size_t buffer_size_in_entries)
@ -21,27 +20,27 @@ KResult KCOVInstance::buffer_allocate(size_t buffer_size_in_entries)
return EINVAL; return EINVAL;
// first entry contains index of last PC // first entry contains index of last PC
this->m_buffer_size_in_entries = buffer_size_in_entries - 1; m_buffer_size_in_entries = buffer_size_in_entries - 1;
this->m_buffer_size_in_bytes = Memory::page_round_up(buffer_size_in_entries * KCOV_ENTRY_SIZE); m_buffer_size_in_bytes = Memory::page_round_up(buffer_size_in_entries * KCOV_ENTRY_SIZE);
// one single vmobject is representing the buffer // one single vmobject is representing the buffer
// - we allocate one kernel region using that vmobject // - we allocate one kernel region using that vmobject
// - when an mmap call comes in, we allocate another userspace region, // - when an mmap call comes in, we allocate another userspace region,
// backed by the same vmobject // backed by the same vmobject
auto maybe_vmobject = Memory::AnonymousVMObject::try_create_with_size( auto maybe_vmobject = Memory::AnonymousVMObject::try_create_with_size(
this->m_buffer_size_in_bytes, AllocationStrategy::AllocateNow); m_buffer_size_in_bytes, AllocationStrategy::AllocateNow);
if (maybe_vmobject.is_error()) if (maybe_vmobject.is_error())
return maybe_vmobject.error(); return maybe_vmobject.error();
this->vmobject = maybe_vmobject.release_value(); m_vmobject = maybe_vmobject.release_value();
this->m_kernel_region = MM.allocate_kernel_region_with_vmobject( m_kernel_region = MM.allocate_kernel_region_with_vmobject(
*this->vmobject, this->m_buffer_size_in_bytes, String::formatted("kcov_{}", this->m_pid), *m_vmobject, m_buffer_size_in_bytes, String::formatted("kcov_{}", m_pid),
Memory::Region::Access::ReadWrite); Memory::Region::Access::ReadWrite);
if (!this->m_kernel_region) if (!m_kernel_region)
return ENOMEM; return ENOMEM;
this->m_buffer = (u64*)this->m_kernel_region->vaddr().as_ptr(); m_buffer = (u64*)m_kernel_region->vaddr().as_ptr();
if (!this->has_buffer()) if (!has_buffer())
return ENOMEM; return ENOMEM;
return KSuccess; return KSuccess;
@ -49,14 +48,14 @@ KResult KCOVInstance::buffer_allocate(size_t buffer_size_in_entries)
void KCOVInstance::buffer_add_pc(u64 pc) void KCOVInstance::buffer_add_pc(u64 pc)
{ {
auto idx = (u64)this->m_buffer[0]; auto idx = (u64)m_buffer[0];
if (idx >= this->m_buffer_size_in_entries) { if (idx >= m_buffer_size_in_entries) {
// the buffer is already full // the buffer is already full
return; return;
} }
this->m_buffer[idx + 1] = pc; m_buffer[idx + 1] = pc;
this->m_buffer[0] = idx + 1; m_buffer[0] = idx + 1;
} }
} }

23
Kernel/Devices/KCOVInstance.h
View file

@ -35,23 +35,30 @@ public:
bool has_buffer() const { return m_buffer != nullptr; } bool has_buffer() const { return m_buffer != nullptr; }
void buffer_add_pc(u64 pc); void buffer_add_pc(u64 pc);
Spinlock lock; enum State {
enum {
UNUSED = 0, UNUSED = 0,
OPENED = 1, OPENED = 1,
TRACING = 2, TRACING = 2,
} state; } m_state { UNUSED };
RefPtr<Memory::AnonymousVMObject> vmobject; State state() const { return m_state; }
void set_state(State state) { m_state = state; }
Memory::VMObject* vmobject() { return m_vmobject; }
Spinlock& spinlock() { return m_lock; }
private: private:
ProcessID m_pid = { 0 }; ProcessID m_pid { 0 };
u64 m_buffer_size_in_entries = { 0 }; u64 m_buffer_size_in_entries { 0 };
size_t m_buffer_size_in_bytes = { 0 }; size_t m_buffer_size_in_bytes { 0 };
kcov_pc_t* m_buffer = { nullptr }; kcov_pc_t* m_buffer { nullptr };
RefPtr<Memory::AnonymousVMObject> m_vmobject;
// Here to ensure it's not garbage collected at the end of open() // Here to ensure it's not garbage collected at the end of open()
OwnPtr<Memory::Region> m_kernel_region; OwnPtr<Memory::Region> m_kernel_region;
Spinlock m_lock;
}; };
} }

2
Kernel/SanCov.cpp
View file

@ -30,7 +30,7 @@ void __sanitizer_cov_trace_pc(void)
return; return;
} }
auto kcov_instance = maybe_kcov_instance.value(); auto kcov_instance = maybe_kcov_instance.value();
if (kcov_instance->state < KCOVInstance::TRACING) [[likely]] if (kcov_instance->state() < KCOVInstance::TRACING) [[likely]]
return; return;
kcov_instance->buffer_add_pc((u64)__builtin_return_address(0)); kcov_instance->buffer_add_pc((u64)__builtin_return_address(0));
} }