Follow practices suggested by "Effective C++"

The patch adjust the code of the base, base-<kernel>, and os repository.
To adapt existing components to fix violations of the best practices
suggested by "Effective C++" as reported by the -Weffc++ compiler
argument. The changes follow the patterns outlined below:

* A class with virtual functions can no longer publicly inherit base
  classed without a vtable. The inherited object may either be moved
  to a member variable, or inherited privately. The latter would be
  used for classes that inherit 'List::Element' or 'Avl_node'. In order
  to enable the 'List' and 'Avl_tree' to access the meta data, the
  'List' must become a friend.

* Instead of adding a virtual destructor to abstract base classes,
  we inherit the new 'Interface' class, which contains a virtual
  destructor. This way, single-line abstract base classes can stay
  as compact as they are now. The 'Interface' utility resides in
  base/include/util/interface.h.

* With the new warnings enabled, all member variables must be explicitly
  initialized. Basic types may be initialized with '='. All other types
  are initialized with braces '{ ... }' or as class initializers. If
  basic types and non-basic types appear in a row, it is nice to only
  use the brace syntax (also for basic types) and align the braces.

* If a class contains pointers as members, it must now also provide a
  copy constructor and assignment operator. In the most cases, one
  would make them private, effectively disallowing the objects to be
  copied. Unfortunately, this warning cannot be fixed be inheriting
  our existing 'Noncopyable' class (the compiler fails to detect that
  the inheriting class cannot be copied and still gives the error).
  For now, we have to manually add declarations for both the copy
  constructor and assignment operator as private class members. Those
  declarations should be prepended with a comment like this:

        /*
         * Noncopyable
         */
        Thread(Thread const &);
        Thread &operator = (Thread const &);

  In the future, we should revisit these places and try to replace
  the pointers with references. In the presence of at least one
  reference member, the compiler would no longer implicitly generate
  a copy constructor. So we could remove the manual declaration.

Issue #465
This commit is contained in:
Norman Feske
2017-12-21 15:42:15 +01:00
committed by Christian Helmuth
parent 2a33d9aa76
commit eba9c15746
763 changed files with 4936 additions and 3288 deletions

View File

@ -22,7 +22,7 @@ namespace Genode { class Core_region_map; }
struct Genode::Core_region_map : Region_map_mmap
{
Core_region_map(Rpc_entrypoint &ep) : Region_map_mmap(false) { }
Core_region_map(Rpc_entrypoint &) : Region_map_mmap(false) { }
};
#endif /* _CORE__INCLUDE__CORE_REGION_MAP_H_ */

View File

@ -32,17 +32,17 @@ namespace Genode {
/**
* Deriving classes can own a dataspace to implement conditional behavior
*/
class Dataspace_owner { };
class Dataspace_owner : Interface { };
class Dataspace_component : public Rpc_object<Linux_dataspace>
{
private:
Filename _fname; /* filename for mmap */
size_t _size; /* size of dataspace in bytes */
addr_t _addr; /* meaningless on linux */
int _fd; /* file descriptor */
bool _writable; /* false if read-only */
Filename _fname { }; /* filename for mmap */
size_t _size { 0 }; /* size of dataspace in bytes */
addr_t _addr { 0 }; /* meaningless on linux */
int _fd { -1 }; /* file descriptor */
bool _writable { false }; /* false if read-only */
/* Holds the dataspace owner if a distinction between owner and
* others is necessary on the dataspace, otherwise it is 0 */
@ -51,6 +51,12 @@ namespace Genode {
static Filename _file_name(const char *args);
size_t _file_size();
/*
* Noncopyable
*/
Dataspace_component(Dataspace_component const &);
Dataspace_component &operator = (Dataspace_component const &);
public:
/**
@ -72,9 +78,8 @@ namespace Genode {
* This constructor is only provided for compatibility
* reasons and should not be used.
*/
Dataspace_component(size_t size, addr_t core_local_addr,
addr_t phys_addr, Cache_attribute,
bool writable, Dataspace_owner * _owner)
Dataspace_component(size_t size, addr_t, addr_t phys_addr,
Cache_attribute, bool, Dataspace_owner *_owner)
:
_size(size), _addr(phys_addr), _fd(-1), _owner(_owner)
{

View File

@ -23,32 +23,34 @@ namespace Genode {
}
class Genode::Irq_session_component : public Rpc_object<Irq_session>,
public List<Irq_session_component>::Element
private List<Irq_session_component>::Element
{
private:
friend class List<Irq_session_component>;
public:
/**
* Constructor
*
* \param irq_alloc platform-dependent IRQ allocator
* \param args session construction arguments
*/
Irq_session_component(Range_allocator *irq_alloc,
const char *args) { }
Irq_session_component(Range_allocator *, const char *) { }
/**
* Destructor
*/
~Irq_session_component() { }
/***************************
** Irq session interface **
***************************/
void ack_irq() override { }
void sigh(Signal_context_capability) override { }
Info info() override {
return { .type = Genode::Irq_session::Info::Type::INVALID }; }
Info info() override {
return { .type = Genode::Irq_session::Info::Type::INVALID,
.address = 0, .value = 0 }; }
};
#endif /* _CORE__INCLUDE__IRQ_SESSION_COMPONENT_H_ */

View File

@ -29,9 +29,8 @@ namespace Genode {
struct Pager_object
{
Thread_capability _thread_cap;
Signal_context_capability _sigh;
Thread_capability _thread_cap { };
Signal_context_capability _sigh { };
virtual ~Pager_object() { }
@ -41,7 +40,7 @@ namespace Genode {
* Remember thread cap so that rm_session can tell thread that
* rm_client is gone.
*/
Thread_capability thread_cap() { return _thread_cap; } const
Thread_capability thread_cap() const { return _thread_cap; }
void thread_cap(Thread_capability cap) { _thread_cap = cap; }
};

View File

@ -41,7 +41,7 @@ namespace Genode {
*/
struct Pseudo_ram_allocator : Range_allocator
{
bool alloc(size_t size, void **out_addr)
bool alloc(size_t, void **out_addr)
{
*out_addr = 0;
return true;
@ -69,7 +69,7 @@ namespace Genode {
bool need_size_for_free() const override { return true; }
};
Pseudo_ram_allocator _ram_alloc;
Pseudo_ram_allocator _ram_alloc { };
public:

View File

@ -45,8 +45,8 @@ namespace Genode {
struct Registry
{
Lock _lock;
List<Platform_thread> _list;
Lock _lock { };
List<Platform_thread> _list { };
void insert(Platform_thread *thread);
void remove(Platform_thread *thread);
@ -62,27 +62,27 @@ namespace Genode {
*/
static Registry *_registry();
unsigned long _tid;
unsigned long _pid;
char _name[32];
unsigned long _tid = -1;
unsigned long _pid = -1;
char _name[32] { };
/**
* Unix-domain socket pair bound to the thread
*/
Socket_pair _socket_pair;
Socket_pair _socket_pair { };
/*
* Dummy pager object that is solely used for storing the
* 'Signal_context_capability' for the thread's exception handler.
*/
Pager_object _pager;
Pager_object _pager { };
public:
/**
* Constructor
*/
Platform_thread(size_t, const char *name, unsigned priority,
Platform_thread(size_t, const char *name, unsigned priority,
Affinity::Location, addr_t);
~Platform_thread();
@ -112,7 +112,7 @@ namespace Genode {
*/
Pager_object *pager() { return &_pager; }
void pager(Pager_object *) { }
int start(void *ip, void *sp) { return 0; }
int start(void *, void *) { return 0; }
Thread_state state()
{
@ -170,7 +170,7 @@ namespace Genode {
/**
* Set CPU quota of the thread to 'quota'
*/
void quota(size_t const quota) { /* not supported*/ }
void quota(size_t const) { /* not supported*/ }
/**
* Return execution time consumed by the thread

View File

@ -35,10 +35,12 @@ namespace Genode {
class Genode::Region_map_component : public Rpc_object<Region_map>,
public List<Region_map_component>::Element
private List<Region_map_component>::Element
{
private:
friend class List<Region_map_component>;
struct Rm_dataspace_component { void sub_rm(Native_capability) { } };
public:
@ -46,7 +48,7 @@ class Genode::Region_map_component : public Rpc_object<Region_map>,
Region_map_component(Rpc_entrypoint &, Allocator &, Pager_entrypoint &,
addr_t, size_t, Session::Diag) { }
void upgrade_ram_quota(size_t ram_quota) { }
void upgrade_ram_quota(size_t) { }
void add_client(Rm_client &) { }
void remove_client(Rm_client &) { }
@ -68,14 +70,17 @@ class Genode::Region_map_component : public Rpc_object<Region_map>,
};
struct Genode::Rm_member { Region_map_component *member_rm() { return 0; } };
struct Genode::Rm_member : Interface
{
Region_map_component *member_rm() { return 0; }
};
struct Genode::Rm_client : Pager_object, Rm_member
{
Rm_client(Cpu_session_capability, Thread_capability,
Region_map_component *rm, unsigned long badge,
Affinity::Location location, Cpu_session::Name const&,
Region_map_component *, unsigned long,
Affinity::Location, Cpu_session::Name const&,
Session_label const&)
{ }
};

View File

@ -29,7 +29,7 @@ class Genode::Rpc_cap_factory
public:
Rpc_cap_factory(Allocator &md_alloc) { }
Rpc_cap_factory(Allocator &) { }
Native_capability alloc(Native_capability ep);

View File

@ -33,8 +33,9 @@
struct Uds_addr : sockaddr_un
{
Uds_addr(long thread_id)
:
sockaddr_un({.sun_family = AF_UNIX, .sun_path = { }})
{
sun_family = AF_UNIX;
Genode::snprintf(sun_path, sizeof(sun_path), "%s/ep-%ld",
resource_path(), thread_id);
}

View File

@ -18,8 +18,8 @@
using namespace Genode;
Io_mem_session_component::Io_mem_session_component(Range_allocator *io_mem_alloc,
Range_allocator *ram_alloc,
Rpc_entrypoint *ds_ep,
Io_mem_session_component::Io_mem_session_component(Range_allocator *,
Range_allocator *,
Rpc_entrypoint *,
const char *args) {
warning("no io_mem support on Linux (args=\"", args, "\")"); }

View File

@ -179,10 +179,9 @@ void Native_pd_component::_start(Dataspace_component &ds)
}
Native_pd_component::Native_pd_component(Pd_session_component &pd_session,
const char *args)
Native_pd_component::Native_pd_component(Pd_session_component &pd, const char *)
:
_pd_session(pd_session)
_pd_session(pd)
{
_pd_session._ep.manage(this);
}

View File

@ -75,14 +75,14 @@ static Pipe_semaphore _wait_for_exit_sem; /* wakeup of '_wait_for_exit' */
static bool _do_exit = false; /* exit condition */
static void sigint_handler(int signum)
static void sigint_handler(int)
{
_do_exit = true;
_wait_for_exit_sem.up();
}
static void sigchld_handler(int signnum)
static void sigchld_handler(int)
{
_wait_for_exit_sem.up();
}

View File

@ -76,7 +76,6 @@ Platform_thread::Registry *Platform_thread::_registry()
Platform_thread::Platform_thread(size_t, const char *name, unsigned,
Affinity::Location, addr_t)
: _tid(-1), _pid(-1)
{
strncpy(_name, name, min(sizeof(_name), strlen(name) + 1));

View File

@ -64,4 +64,4 @@ void Ram_dataspace_factory::_revoke_ram_ds(Dataspace_component *ds)
}
void Ram_dataspace_factory::_clear_ds(Dataspace_component *ds) { }
void Ram_dataspace_factory::_clear_ds(Dataspace_component *) { }

View File

@ -33,7 +33,7 @@
using namespace Genode;
Rom_session_component::Rom_session_component(Rom_fs *rom_fs,
Rom_session_component::Rom_session_component(Rom_fs *,
Rpc_entrypoint *ds_ep,
const char *args)
: _ds(args), _ds_ep(ds_ep)

View File

@ -42,10 +42,8 @@ class Stack_area_region_map : public Genode::Region_map
/**
* Attach backing store to stack area
*/
Local_addr attach(Genode::Dataspace_capability ds_cap,
Genode::size_t size, Genode::off_t offset,
bool use_local_addr, Local_addr local_addr,
bool executable)
Local_addr attach(Genode::Dataspace_capability, Genode::size_t size,
Genode::off_t, bool, Local_addr local_addr, bool)
{
using namespace Genode;
@ -80,7 +78,7 @@ class Stack_area_region_map : public Genode::Region_map
struct Stack_area_ram_allocator : Genode::Ram_allocator
{
Genode::Ram_dataspace_capability alloc(Genode::size_t size,
Genode::Ram_dataspace_capability alloc(Genode::size_t,
Genode::Cache_attribute) override {
return Genode::Ram_dataspace_capability(); }

View File

@ -46,7 +46,7 @@ class Genode::Local_parent : public Expanding_parent_client
private:
Allocator &_alloc;
Id_space<Client> _local_sessions_id_space;
Id_space<Client> _local_sessions_id_space { };
public:

View File

@ -46,10 +46,7 @@ static inline bool thread_check_stopped_and_restart(Genode::Thread *thread_base)
}
static inline void thread_switch_to(Genode::Thread *thread_base)
{
thread_yield();
}
static inline void thread_switch_to(Genode::Thread *) { thread_yield(); }
static inline void thread_stop_myself()

View File

@ -46,7 +46,7 @@ struct Genode::Native_thread
*/
Meta_data *meta_data = nullptr;
Socket_pair socket_pair;
Socket_pair socket_pair { };
Native_thread() { }
};

View File

@ -35,7 +35,7 @@ class Genode::Region_map_mmap : public Region_map, public Dataspace
{
private:
Region_registry _rmap;
Region_registry _rmap { };
bool const _sub_rm; /* false if region map is root */
size_t const _size;
@ -120,7 +120,7 @@ class Genode::Region_map_mmap : public Region_map, public Dataspace
void detach(Local_addr local_addr);
void fault_handler(Signal_context_capability handler) { }
void fault_handler(Signal_context_capability) { }
State state() { return State(); }

View File

@ -27,10 +27,10 @@ class Genode::Region
{
private:
addr_t _start;
off_t _offset;
Dataspace_capability _ds;
size_t _size;
addr_t _start { 0 };
off_t _offset { 0 };
Dataspace_capability _ds { };
size_t _size { 0 };
/**
* Return offset of first byte after the region
@ -39,7 +39,7 @@ class Genode::Region
public:
Region() : _start(0), _offset(0), _size(0) { }
Region() { }
Region(addr_t start, off_t offset, Dataspace_capability ds, size_t size)
: _start(start), _offset(offset), _ds(ds), _size(size) { }

View File

@ -66,7 +66,7 @@ class Genode::Socket_descriptor_registry
Entry _entries[MAX_FDS];
Genode::Lock mutable _lock;
Genode::Lock mutable _lock { };
Entry &_find_free_entry()
{

View File

@ -60,7 +60,7 @@ Child::Process::Loaded_executable::Loaded_executable(Dataspace_capability,
Child::Process::Process(Dataspace_capability elf_ds,
Dataspace_capability ldso_ds,
Pd_session_capability pd_cap,
Pd_session_capability,
Pd_session &pd,
Ram_session &ram,
Initial_thread_base &,

View File

@ -182,9 +182,9 @@ namespace {
typedef Genode::size_t size_t;
msghdr _msg;
sockaddr_un _addr;
iovec _iovec;
msghdr _msg { };
sockaddr_un _addr { };
iovec _iovec { };
char _cmsg_buf[CMSG_SPACE(MAX_SDS_PER_MSG*sizeof(int))];
unsigned _num_sds;
@ -286,7 +286,7 @@ static void extract_sds_from_message(unsigned start_index,
Genode::Msgbuf_base &buf)
{
unsigned sd_cnt = 0;
for (unsigned i = 0; i < min(header.num_caps, Msgbuf_base::MAX_CAPS_PER_MSG); i++) {
for (unsigned i = 0; i < min(header.num_caps, (size_t)Msgbuf_base::MAX_CAPS_PER_MSG); i++) {
unsigned long const badge = header.badges[i];

View File

@ -83,7 +83,7 @@ void Thread::_thread_start()
}
void Thread::_init_platform_thread(size_t weight, Type type)
void Thread::_init_platform_thread(size_t /* weight */, Type type)
{
/* if no cpu session is given, use it from the environment */
if (!_cpu_session)

View File

@ -35,7 +35,7 @@ enum { verbose_atexit = false };
/**
* Dummy for symbol that is normally provided by '_main.cc'
*/
int genode___cxa_atexit(void (*func)(void*), void *arg, void *dso)
int genode___cxa_atexit(void (*)(void*), void *, void *)
{
if (verbose_atexit)
Genode::raw("genode___cxa_atexit called, not implemented\n");
@ -146,7 +146,9 @@ int main()
/* host libc includes */
#define size_t __SIZE_TYPE__ /* see comment in 'linux_syscalls.h' */
#pragma GCC diagnostic ignored "-Weffc++"
#include <pthread.h>
#pragma GCC diagnostic pop
#include <stdio.h>
#include <errno.h>
#undef size_t
@ -161,7 +163,7 @@ static pthread_key_t tls_key()
{
struct Tls_key
{
pthread_key_t key;
pthread_key_t key { };
Tls_key()
{
@ -188,29 +190,31 @@ namespace Genode {
* 'Stack'. But the POSIX threads of hybrid programs have no 'Stack'
* object. So we have to keep the meta data here.
*/
Native_thread native_thread;
Native_thread native_thread { };
/**
* Filled out by 'thread_start' function in the stack of the new
* thread
*/
Thread * const thread_base;
Thread &thread_base;
/**
* POSIX thread handle
*/
pthread_t pt;
pthread_t pt { };
/**
* Constructor
*
* \param thread associated 'Thread' object
*/
Meta_data(Thread *thread) : thread_base(thread)
Meta_data(Thread *thread) : thread_base(*thread)
{
native_thread.meta_data = this;
}
virtual ~Meta_data() { }
/**
* Used to block the constructor until the new thread has initialized
* 'id'
@ -247,17 +251,17 @@ namespace Genode {
* Used to block the constructor until the new thread has initialized
* 'id'
*/
Barrier _construct_lock;
Barrier _construct_lock { };
/**
* Used to block the new thread until 'start' is called
*/
Barrier _start_lock;
Barrier _start_lock { };
/**
* Used to block the 'join()' function until the 'entry()' is done
*/
Barrier _join_lock;
Barrier _join_lock { };
public:
@ -362,7 +366,7 @@ static void adopt_thread(Native_thread::Meta_data *meta_data)
/*
* Initialize thread meta data
*/
Native_thread &native_thread = meta_data->thread_base->native_thread();
Native_thread &native_thread = meta_data->thread_base.native_thread();
native_thread.tid = lx_gettid();
native_thread.pid = lx_getpid();
}
@ -402,11 +406,11 @@ namespace {
return true;
}
void free(void *addr, size_t size) override { ::free(addr); }
void free(void *addr, size_t) override { ::free(addr); }
bool need_size_for_free() const override { return false; }
size_t overhead(size_t size) const override { return 0; }
size_t overhead(size_t) const override { return 0; }
};
}
@ -423,7 +427,7 @@ Thread *Thread::myself()
void * const tls = pthread_getspecific(tls_key());
if (tls != 0)
return ((Native_thread::Meta_data *)tls)->thread_base;
return &((Native_thread::Meta_data *)tls)->thread_base;
bool const called_by_main_thread = (lx_getpid() == lx_gettid());
if (called_by_main_thread)
@ -457,7 +461,7 @@ Thread *Thread::myself()
* Initialize 'Thread::_native_thread' to point to the default-
* constructed 'Native_thread' (part of 'Meta_data').
*/
meta_data->thread_base->_native_thread = &meta_data->native_thread;
meta_data->thread_base._native_thread = &meta_data->native_thread;
adopt_thread(meta_data);
return thread;
@ -482,9 +486,9 @@ void Thread::join()
Native_thread &Thread::native_thread() { return *_native_thread; }
Thread::Thread(size_t weight, const char *name, size_t stack_size,
Type type, Cpu_session * cpu_sess, Affinity::Location)
: _cpu_session(cpu_sess)
Thread::Thread(size_t weight, const char *name, size_t /* stack size */,
Type, Cpu_session * cpu_sess, Affinity::Location)
: _cpu_session(cpu_sess), _affinity(), _join_lock()
{
Native_thread::Meta_data *meta_data =
new (global_alloc()) Thread_meta_data_created(this);
@ -513,7 +517,7 @@ Thread::Thread(size_t weight, const char *name, size_t stack_size,
: Thread(weight, name, stack_size, type, &_env_ptr->cpu()) { }
Thread::Thread(Env &env, Name const &name, size_t stack_size, Location location,
Thread::Thread(Env &, Name const &name, size_t stack_size, Location location,
Weight weight, Cpu_session &cpu)
: Thread(weight.value, name.string(), stack_size, NORMAL,
&cpu, location)

View File

@ -17,7 +17,9 @@
#include <base/log.h>
/* libc includes */
#pragma GCC diagnostic ignored "-Weffc++"
#include <pthread.h>
#pragma GCC diagnostic pop
#include <stdlib.h>