mirror of
https://github.com/SerenityOS/serenity
synced 2024-09-06 08:56:40 +00:00
Everywhere: Prevent risky implicit casts of (Nonnull)RefPtr
Our existing implementation did not check the element type of the other pointer in the constructors and move assignment operators. This meant that some operations that would require explicit casting on raw pointers were done implicitly, such as: - downcasting a base class to a derived class (e.g. `Kernel::Inode` => `Kernel::ProcFSDirectoryInode` in Kernel/ProcFS.cpp), - casting to an unrelated type (e.g. `Promise<bool>` => `Promise<Empty>` in LibIMAP/Client.cpp) This, of course, allows gross violations of the type system, and makes the need to type-check less obvious before downcasting. Luckily, while adding the `static_ptr_cast`s, only two truly incorrect usages were found; in the other instances, our casts just needed to be made explicit.
This commit is contained in:
parent
bad23e3f8c
commit
d7b6cc6421
|
@ -58,7 +58,7 @@ public:
|
|||
const_cast<T&>(object).ref();
|
||||
}
|
||||
template<typename U>
|
||||
ALWAYS_INLINE NonnullRefPtr(const U& object)
|
||||
ALWAYS_INLINE NonnullRefPtr(const U& object) requires(IsConvertible<U*, T*>)
|
||||
: m_bits((FlatPtr) static_cast<const T*>(&object))
|
||||
{
|
||||
VERIFY(!(m_bits & 1));
|
||||
|
@ -75,7 +75,7 @@ public:
|
|||
VERIFY(!(m_bits & 1));
|
||||
}
|
||||
template<typename U>
|
||||
ALWAYS_INLINE NonnullRefPtr(NonnullRefPtr<U>&& other)
|
||||
ALWAYS_INLINE NonnullRefPtr(NonnullRefPtr<U>&& other) requires(IsConvertible<U*, T*>)
|
||||
: m_bits((FlatPtr)&other.leak_ref())
|
||||
{
|
||||
VERIFY(!(m_bits & 1));
|
||||
|
@ -86,7 +86,7 @@ public:
|
|||
VERIFY(!(m_bits & 1));
|
||||
}
|
||||
template<typename U>
|
||||
ALWAYS_INLINE NonnullRefPtr(const NonnullRefPtr<U>& other)
|
||||
ALWAYS_INLINE NonnullRefPtr(const NonnullRefPtr<U>& other) requires(IsConvertible<U*, T*>)
|
||||
: m_bits((FlatPtr)other.add_ref())
|
||||
{
|
||||
VERIFY(!(m_bits & 1));
|
||||
|
@ -119,7 +119,7 @@ public:
|
|||
}
|
||||
|
||||
template<typename U>
|
||||
NonnullRefPtr& operator=(const NonnullRefPtr<U>& other)
|
||||
NonnullRefPtr& operator=(const NonnullRefPtr<U>& other) requires(IsConvertible<U*, T*>)
|
||||
{
|
||||
assign(other.add_ref());
|
||||
return *this;
|
||||
|
@ -133,7 +133,7 @@ public:
|
|||
}
|
||||
|
||||
template<typename U>
|
||||
NonnullRefPtr& operator=(NonnullRefPtr<U>&& other)
|
||||
NonnullRefPtr& operator=(NonnullRefPtr<U>&& other) requires(IsConvertible<U*, T*>)
|
||||
{
|
||||
assign(&other.leak_ref());
|
||||
return *this;
|
||||
|
@ -213,7 +213,7 @@ public:
|
|||
}
|
||||
|
||||
template<typename U>
|
||||
void swap(NonnullRefPtr<U>& other)
|
||||
void swap(NonnullRefPtr<U>& other) requires(IsConvertible<U*, T*>)
|
||||
{
|
||||
// NOTE: swap is not atomic!
|
||||
U* other_ptr = other.exchange(nullptr);
|
||||
|
@ -327,7 +327,7 @@ struct Formatter<NonnullRefPtr<T>> : Formatter<const T*> {
|
|||
};
|
||||
|
||||
template<typename T, typename U>
|
||||
inline void swap(NonnullRefPtr<T>& a, NonnullRefPtr<U>& b)
|
||||
inline void swap(NonnullRefPtr<T>& a, NonnullRefPtr<U>& b) requires(IsConvertible<U*, T*>)
|
||||
{
|
||||
a.swap(b);
|
||||
}
|
||||
|
|
20
AK/RefPtr.h
20
AK/RefPtr.h
|
@ -154,18 +154,18 @@ public:
|
|||
{
|
||||
}
|
||||
template<typename U>
|
||||
ALWAYS_INLINE RefPtr(const NonnullRefPtr<U>& other)
|
||||
ALWAYS_INLINE RefPtr(const NonnullRefPtr<U>& other) requires(IsConvertible<U*, T*>)
|
||||
: m_bits(PtrTraits::as_bits(const_cast<U*>(other.add_ref())))
|
||||
{
|
||||
}
|
||||
template<typename U>
|
||||
ALWAYS_INLINE RefPtr(NonnullRefPtr<U>&& other)
|
||||
ALWAYS_INLINE RefPtr(NonnullRefPtr<U>&& other) requires(IsConvertible<U*, T*>)
|
||||
: m_bits(PtrTraits::as_bits(&other.leak_ref()))
|
||||
{
|
||||
VERIFY(!is_null());
|
||||
}
|
||||
template<typename U, typename P = RefPtrTraits<U>>
|
||||
RefPtr(RefPtr<U, P>&& other)
|
||||
RefPtr(RefPtr<U, P>&& other) requires(IsConvertible<U*, T*>)
|
||||
: m_bits(PtrTraits::template convert_from<U, P>(other.leak_ref_raw()))
|
||||
{
|
||||
}
|
||||
|
@ -174,7 +174,7 @@ public:
|
|||
{
|
||||
}
|
||||
template<typename U, typename P = RefPtrTraits<U>>
|
||||
RefPtr(const RefPtr<U, P>& other)
|
||||
RefPtr(const RefPtr<U, P>& other) requires(IsConvertible<U*, T*>)
|
||||
: m_bits(other.add_ref_raw())
|
||||
{
|
||||
}
|
||||
|
@ -203,7 +203,7 @@ public:
|
|||
}
|
||||
|
||||
template<typename U, typename P = RefPtrTraits<U>>
|
||||
void swap(RefPtr<U, P>& other)
|
||||
void swap(RefPtr<U, P>& other) requires(IsConvertible<U*, T*>)
|
||||
{
|
||||
// NOTE: swap is not atomic!
|
||||
FlatPtr other_bits = P::exchange(other.m_bits, P::default_null_value);
|
||||
|
@ -219,14 +219,14 @@ public:
|
|||
}
|
||||
|
||||
template<typename U, typename P = RefPtrTraits<U>>
|
||||
ALWAYS_INLINE RefPtr& operator=(RefPtr<U, P>&& other)
|
||||
ALWAYS_INLINE RefPtr& operator=(RefPtr<U, P>&& other) requires(IsConvertible<U*, T*>)
|
||||
{
|
||||
assign_raw(PtrTraits::template convert_from<U, P>(other.leak_ref_raw()));
|
||||
return *this;
|
||||
}
|
||||
|
||||
template<typename U>
|
||||
ALWAYS_INLINE RefPtr& operator=(NonnullRefPtr<U>&& other)
|
||||
ALWAYS_INLINE RefPtr& operator=(NonnullRefPtr<U>&& other) requires(IsConvertible<U*, T*>)
|
||||
{
|
||||
assign_raw(PtrTraits::as_bits(&other.leak_ref()));
|
||||
return *this;
|
||||
|
@ -239,7 +239,7 @@ public:
|
|||
}
|
||||
|
||||
template<typename U>
|
||||
ALWAYS_INLINE RefPtr& operator=(const NonnullRefPtr<U>& other)
|
||||
ALWAYS_INLINE RefPtr& operator=(const NonnullRefPtr<U>& other) requires(IsConvertible<U*, T*>)
|
||||
{
|
||||
assign_raw(PtrTraits::as_bits(other.add_ref()));
|
||||
return *this;
|
||||
|
@ -253,7 +253,7 @@ public:
|
|||
}
|
||||
|
||||
template<typename U>
|
||||
ALWAYS_INLINE RefPtr& operator=(const RefPtr<U>& other)
|
||||
ALWAYS_INLINE RefPtr& operator=(const RefPtr<U>& other) requires(IsConvertible<U*, T*>)
|
||||
{
|
||||
assign_raw(other.add_ref_raw());
|
||||
return *this;
|
||||
|
@ -470,7 +470,7 @@ inline RefPtr<T> static_ptr_cast(const RefPtr<U>& ptr)
|
|||
}
|
||||
|
||||
template<typename T, typename PtrTraitsT, typename U, typename PtrTraitsU>
|
||||
inline void swap(RefPtr<T, PtrTraitsT>& a, RefPtr<U, PtrTraitsU>& b)
|
||||
inline void swap(RefPtr<T, PtrTraitsT>& a, RefPtr<U, PtrTraitsU>& b) requires(IsConvertible<U*, T*>)
|
||||
{
|
||||
a.swap(b);
|
||||
}
|
||||
|
|
|
@ -55,7 +55,7 @@ KResult ProcFS::initialize()
|
|||
auto root_inode = ProcFSComponentRegistry::the().root_directory().to_inode(*this);
|
||||
if (root_inode.is_error())
|
||||
return root_inode.error();
|
||||
m_root_inode = static_cast<NonnullRefPtr<ProcFSDirectoryInode>>(root_inode.release_value());
|
||||
m_root_inode = static_ptr_cast<ProcFSDirectoryInode>(root_inode.release_value());
|
||||
|
||||
return KSuccess;
|
||||
}
|
||||
|
|
|
@ -61,12 +61,12 @@ SysFSDirectory::SysFSDirectory(StringView name, SysFSDirectory const& parent_dir
|
|||
{
|
||||
}
|
||||
|
||||
NonnullRefPtr<Inode> SysFSDirectory::to_inode(SysFS const& sysfs_instance) const
|
||||
NonnullRefPtr<SysFSInode> SysFSDirectory::to_inode(SysFS const& sysfs_instance) const
|
||||
{
|
||||
return SysFSDirectoryInode::create(sysfs_instance, *this);
|
||||
}
|
||||
|
||||
NonnullRefPtr<Inode> SysFSComponent::to_inode(SysFS const& sysfs_instance) const
|
||||
NonnullRefPtr<SysFSInode> SysFSComponent::to_inode(SysFS const& sysfs_instance) const
|
||||
{
|
||||
return SysFSInode::create(sysfs_instance, *this);
|
||||
}
|
||||
|
|
|
@ -26,7 +26,7 @@ public:
|
|||
virtual RefPtr<SysFSComponent> lookup(StringView) { VERIFY_NOT_REACHED(); };
|
||||
virtual KResultOr<size_t> write_bytes(off_t, size_t, UserOrKernelBuffer const&, FileDescription*) { return EROFS; }
|
||||
|
||||
virtual NonnullRefPtr<Inode> to_inode(SysFS const&) const;
|
||||
virtual NonnullRefPtr<SysFSInode> to_inode(SysFS const&) const;
|
||||
|
||||
InodeIndex component_index() const { return m_component_index; };
|
||||
|
||||
|
@ -45,7 +45,7 @@ public:
|
|||
virtual KResult traverse_as_directory(unsigned, Function<bool(FileSystem::DirectoryEntryView const&)>) const override;
|
||||
virtual RefPtr<SysFSComponent> lookup(StringView name) override;
|
||||
|
||||
virtual NonnullRefPtr<Inode> to_inode(SysFS const& sysfs_instance) const override final;
|
||||
virtual NonnullRefPtr<SysFSInode> to_inode(SysFS const& sysfs_instance) const override final;
|
||||
|
||||
protected:
|
||||
explicit SysFSDirectory(StringView name);
|
||||
|
|
|
@ -130,7 +130,7 @@ UNMAP_AFTER_INIT bool GraphicsManagement::determine_and_initialize_graphics_devi
|
|||
// Note: If no other VGA adapter is attached as m_vga_adapter, we should attach it then.
|
||||
if (!m_vga_adapter && PCI::is_io_space_enabled(address) && adapter->type() == GraphicsDevice::Type::VGACompatible) {
|
||||
dbgln("Graphics adapter @ {} is operating in VGA mode", address);
|
||||
m_vga_adapter = adapter;
|
||||
m_vga_adapter = static_ptr_cast<VGACompatibleAdapter>(adapter);
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
|
|
@ -27,7 +27,7 @@ KResultOr<FlatPtr> Process::sys$purge(int mode)
|
|||
if (vmobject.is_anonymous()) {
|
||||
// In the event that the append fails, only attempt to continue
|
||||
// the purge if we have already appended something successfully.
|
||||
if (!vmobjects.try_append(vmobject) && vmobjects.is_empty()) {
|
||||
if (!vmobjects.try_append(static_cast<Memory::AnonymousVMObject&>(vmobject)) && vmobjects.is_empty()) {
|
||||
result = ENOMEM;
|
||||
return IterationDecision::Break;
|
||||
}
|
||||
|
|
|
@ -16,7 +16,7 @@
|
|||
#include <LibTest/TestCase.h>
|
||||
|
||||
NonnullRefPtr<SQL::SchemaDef> setup_schema(SQL::Database&);
|
||||
NonnullRefPtr<SQL::SchemaDef> setup_table(SQL::Database&);
|
||||
NonnullRefPtr<SQL::TableDef> setup_table(SQL::Database&);
|
||||
void insert_into_table(SQL::Database&, int);
|
||||
void verify_table_contents(SQL::Database&, int);
|
||||
void insert_and_verify(int);
|
||||
|
@ -28,7 +28,7 @@ NonnullRefPtr<SQL::SchemaDef> setup_schema(SQL::Database& db)
|
|||
return schema;
|
||||
}
|
||||
|
||||
NonnullRefPtr<SQL::SchemaDef> setup_table(SQL::Database& db)
|
||||
NonnullRefPtr<SQL::TableDef> setup_table(SQL::Database& db)
|
||||
{
|
||||
auto schema = setup_schema(db);
|
||||
auto table = SQL::TableDef::construct(schema, "TestTable");
|
||||
|
|
|
@ -220,7 +220,7 @@ void SpreadsheetWidget::setup_tabs(NonnullRefPtrVector<Sheet> new_sheets)
|
|||
};
|
||||
|
||||
m_tab_widget->on_context_menu_request = [&](auto& widget, auto& event) {
|
||||
m_tab_context_menu_sheet_view = widget;
|
||||
m_tab_context_menu_sheet_view = static_cast<SpreadsheetView&>(widget);
|
||||
m_tab_context_menu->popup(event.screen_position());
|
||||
};
|
||||
}
|
||||
|
|
|
@ -13,7 +13,7 @@ void TextureUnit::bind_texture_to_target(GLenum texture_target, const RefPtr<Tex
|
|||
{
|
||||
switch (texture_target) {
|
||||
case GL_TEXTURE_2D:
|
||||
m_texture_target_2d = texture;
|
||||
m_texture_target_2d = static_ptr_cast<Texture2D>(texture);
|
||||
m_currently_bound_texture = texture;
|
||||
m_currently_bound_target = GL_TEXTURE_2D;
|
||||
break;
|
||||
|
|
|
@ -31,7 +31,7 @@ RefPtr<Promise<Empty>> Client::connect()
|
|||
}
|
||||
if (!success)
|
||||
return {};
|
||||
m_connect_pending = Promise<bool>::construct();
|
||||
m_connect_pending = Promise<Empty>::construct();
|
||||
return m_connect_pending;
|
||||
}
|
||||
|
||||
|
|
|
@ -69,7 +69,7 @@ private:
|
|||
// Not yet sent
|
||||
Vector<Command> m_command_queue {};
|
||||
|
||||
RefPtr<Promise<bool>> m_connect_pending {};
|
||||
RefPtr<Promise<Empty>> m_connect_pending {};
|
||||
|
||||
ByteBuffer m_buffer;
|
||||
Parser m_parser;
|
||||
|
|
|
@ -1606,7 +1606,7 @@ NonnullRefPtr<Identifier> Parser::parse_identifier()
|
|||
token.value());
|
||||
}
|
||||
|
||||
NonnullRefPtr<CallExpression> Parser::parse_call_expression(NonnullRefPtr<Expression> lhs)
|
||||
NonnullRefPtr<Expression> Parser::parse_call_expression(NonnullRefPtr<Expression> lhs)
|
||||
{
|
||||
auto rule_start = push_start();
|
||||
if (!m_state.allow_super_constructor_call && is<SuperExpression>(*lhs))
|
||||
|
|
|
@ -85,7 +85,7 @@ public:
|
|||
NonnullRefPtr<StringLiteral> parse_string_literal(const Token& token, bool in_template_literal = false);
|
||||
NonnullRefPtr<TemplateLiteral> parse_template_literal(bool is_tagged);
|
||||
NonnullRefPtr<Expression> parse_secondary_expression(NonnullRefPtr<Expression>, int min_precedence, Associativity associate = Associativity::Right);
|
||||
NonnullRefPtr<CallExpression> parse_call_expression(NonnullRefPtr<Expression>);
|
||||
NonnullRefPtr<Expression> parse_call_expression(NonnullRefPtr<Expression>);
|
||||
NonnullRefPtr<NewExpression> parse_new_expression();
|
||||
NonnullRefPtr<ClassDeclaration> parse_class_declaration();
|
||||
NonnullRefPtr<ClassExpression> parse_class_expression(bool expect_class_name);
|
||||
|
|
|
@ -146,7 +146,7 @@ Value Document::resolve(Value const& value)
|
|||
auto obj = value.as_object();
|
||||
|
||||
if (obj->is_indirect_value())
|
||||
return static_cast<NonnullRefPtr<IndirectValue>>(obj)->value();
|
||||
return static_ptr_cast<IndirectValue>(obj)->value();
|
||||
|
||||
return obj;
|
||||
}
|
||||
|
|
|
@ -248,7 +248,7 @@ template<IsObject To, IsObject From>
|
|||
# undef ENUMERATE_TYPES
|
||||
#endif
|
||||
|
||||
return static_cast<NonnullRefPtr<To>>(obj);
|
||||
return static_ptr_cast<To>(obj);
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -210,7 +210,7 @@ NonnullRefPtr<Insert> Parser::parse_insert_statement(RefPtr<CommonTableExpressio
|
|||
if ((column_names.size() > 0) && (chained_expr->expressions().size() != column_names.size())) {
|
||||
syntax_error("Number of expressions does not match number of columns");
|
||||
} else {
|
||||
chained_expressions.append(chained_expression.release_nonnull());
|
||||
chained_expressions.append(static_ptr_cast<ChainedExpression>(chained_expression.release_nonnull()));
|
||||
}
|
||||
} else {
|
||||
expected("Chained expression");
|
||||
|
|
|
@ -72,7 +72,7 @@ NonnullRefPtr<HTMLTableCaptionElement> HTMLTableElement::create_caption()
|
|||
|
||||
auto caption = DOM::create_element(document(), TagNames::caption, Namespace::HTML);
|
||||
pre_insert(caption, first_child());
|
||||
return caption;
|
||||
return static_ptr_cast<HTMLTableCaptionElement>(caption);
|
||||
}
|
||||
|
||||
void HTMLTableElement::delete_caption()
|
||||
|
@ -159,7 +159,7 @@ NonnullRefPtr<HTMLTableSectionElement> HTMLTableElement::create_t_head()
|
|||
|
||||
pre_insert(thead, child_to_append_after);
|
||||
|
||||
return thead;
|
||||
return static_ptr_cast<HTMLTableSectionElement>(thead);
|
||||
}
|
||||
|
||||
void HTMLTableElement::delete_t_head()
|
||||
|
@ -209,7 +209,7 @@ NonnullRefPtr<HTMLTableSectionElement> HTMLTableElement::create_t_foot()
|
|||
|
||||
auto tfoot = DOM::create_element(document(), TagNames::tfoot, Namespace::HTML);
|
||||
append_child(tfoot);
|
||||
return tfoot;
|
||||
return static_ptr_cast<HTMLTableSectionElement>(tfoot);
|
||||
}
|
||||
|
||||
void HTMLTableElement::delete_t_foot()
|
||||
|
@ -248,7 +248,7 @@ NonnullRefPtr<HTMLTableSectionElement> HTMLTableElement::create_t_body()
|
|||
|
||||
pre_insert(tbody, child_to_append_after);
|
||||
|
||||
return tbody;
|
||||
return static_ptr_cast<HTMLTableSectionElement>(tbody);
|
||||
}
|
||||
|
||||
NonnullRefPtr<DOM::HTMLCollection> HTMLTableElement::rows()
|
||||
|
@ -287,7 +287,7 @@ DOM::ExceptionOr<NonnullRefPtr<HTMLTableRowElement>> HTMLTableElement::insert_ro
|
|||
if (index < -1 || index >= (long)rows_length) {
|
||||
return DOM::IndexSizeError::create("Index is negative or greater than the number of rows");
|
||||
}
|
||||
auto tr = static_cast<NonnullRefPtr<HTMLTableRowElement>>(DOM::create_element(document(), TagNames::tr, Namespace::HTML));
|
||||
auto tr = static_ptr_cast<HTMLTableRowElement>(DOM::create_element(document(), TagNames::tr, Namespace::HTML));
|
||||
if (rows_length == 0 && !has_child_of_type<HTMLTableRowElement>()) {
|
||||
auto tbody = DOM::create_element(document(), TagNames::tbody, Namespace::HTML);
|
||||
tbody->append_child(tr);
|
||||
|
|
|
@ -166,7 +166,7 @@ void TreeBuilder::remove_irrelevant_boxes(NodeWithStyle& root)
|
|||
{
|
||||
// The following boxes are discarded as if they were display:none:
|
||||
|
||||
NonnullRefPtrVector<Box> to_remove;
|
||||
NonnullRefPtrVector<Node> to_remove;
|
||||
|
||||
// Children of a table-column.
|
||||
for_each_in_tree_with_display<CSS::Display::TableColumn>(root, [&](Box& table_column) {
|
||||
|
|
|
@ -1431,7 +1431,7 @@ RefPtr<AST::Node> Parser::parse_variable_ref()
|
|||
return create<AST::SimpleVariable>(move(name)); // Variable Simple
|
||||
}
|
||||
|
||||
RefPtr<AST::Node> Parser::parse_slice()
|
||||
RefPtr<AST::Slice> Parser::parse_slice()
|
||||
{
|
||||
auto rule_start = push_start();
|
||||
if (!next_is("["))
|
||||
|
|
|
@ -85,7 +85,7 @@ private:
|
|||
RefPtr<AST::Node> parse_string_inner(StringEndCondition);
|
||||
RefPtr<AST::Node> parse_variable();
|
||||
RefPtr<AST::Node> parse_variable_ref();
|
||||
RefPtr<AST::Node> parse_slice();
|
||||
RefPtr<AST::Slice> parse_slice();
|
||||
RefPtr<AST::Node> parse_evaluate();
|
||||
RefPtr<AST::Node> parse_history_designator();
|
||||
RefPtr<AST::Node> parse_comment();
|
||||
|
|
Loading…
Reference in a new issue