LibIPC: Replace Encoder::encode methods with IPC::encode specializations

Currently, the stream operator overload hides most encoding errors. In
an effort to make IPC encoding fallible, this first replaces the Encoder
overloads with IPC::encode specializations. The return type is still a
boolean, a future commit will change it to ErrorOr.

Note that just like in the analogous decoder commit (9b48362), these
specializations must be defined at the namespace scope. Further, all
arithmetic specializations are now in one method.

Userland/Libraries/LibIPC/Encoder.cpp

@@ -20,170 +20,81 @@
 
 namespace IPC {
 
-Encoder& Encoder::operator<<(bool value)
-{
-    return *this << (u8)value;
-}
-
-Encoder& Encoder::operator<<(u8 value)
-{
-    m_buffer.data.append(value);
-    return *this;
-}
-
-Encoder& Encoder::operator<<(u16 value)
-{
-    m_buffer.data.ensure_capacity(m_buffer.data.size() + 2);
-    m_buffer.data.unchecked_append((u8)value);
-    m_buffer.data.unchecked_append((u8)(value >> 8));
-    return *this;
-}
-
-void Encoder::encode_u32(u32 value)
-{
-    m_buffer.data.ensure_capacity(m_buffer.data.size() + 4);
-    m_buffer.data.unchecked_append((u8)value);
-    m_buffer.data.unchecked_append((u8)(value >> 8));
-    m_buffer.data.unchecked_append((u8)(value >> 16));
-    m_buffer.data.unchecked_append((u8)(value >> 24));
-}
-
-void Encoder::encode_u64(u64 value)
-{
-    m_buffer.data.ensure_capacity(m_buffer.data.size() + 8);
-    m_buffer.data.unchecked_append((u8)value);
-    m_buffer.data.unchecked_append((u8)(value >> 8));
-    m_buffer.data.unchecked_append((u8)(value >> 16));
-    m_buffer.data.unchecked_append((u8)(value >> 24));
-    m_buffer.data.unchecked_append((u8)(value >> 32));
-    m_buffer.data.unchecked_append((u8)(value >> 40));
-    m_buffer.data.unchecked_append((u8)(value >> 48));
-    m_buffer.data.unchecked_append((u8)(value >> 56));
-}
-
-Encoder& Encoder::operator<<(unsigned value)
-{
-    encode_u32(value);
-    return *this;
-}
-
-Encoder& Encoder::operator<<(unsigned long value)
-{
-    if constexpr (sizeof(value) == 4)
-        encode_u32(value);
-    else
-        encode_u64(value);
-    return *this;
-}
-
-Encoder& Encoder::operator<<(unsigned long long value)
-{
-    if constexpr (sizeof(value) == 4)
-        encode_u32(value);
-    else
-        encode_u64(value);
-    return *this;
-}
-
-Encoder& Encoder::operator<<(i8 value)
-{
-    m_buffer.data.append((u8)value);
-    return *this;
-}
-
-Encoder& Encoder::operator<<(i16 value)
+template<>
+bool encode(Encoder& encoder, float const& value)
 {
-    m_buffer.data.ensure_capacity(m_buffer.data.size() + 2);
-    m_buffer.data.unchecked_append((u8)value);
-    m_buffer.data.unchecked_append((u8)(value >> 8));
-    return *this;
+    return encoder.encode(bit_cast<u32>(value));
 }
 
-Encoder& Encoder::operator<<(i32 value)
+template<>
+bool encode(Encoder& encoder, double const& value)
 {
-    m_buffer.data.ensure_capacity(m_buffer.data.size() + 4);
-    m_buffer.data.unchecked_append((u8)value);
-    m_buffer.data.unchecked_append((u8)(value >> 8));
-    m_buffer.data.unchecked_append((u8)(value >> 16));
-    m_buffer.data.unchecked_append((u8)(value >> 24));
-    return *this;
+    return encoder.encode(bit_cast<u64>(value));
 }
 
-Encoder& Encoder::operator<<(i64 value)
+template<>
+bool encode(Encoder& encoder, StringView const& value)
 {
-    m_buffer.data.ensure_capacity(m_buffer.data.size() + 8);
-    m_buffer.data.unchecked_append((u8)value);
-    m_buffer.data.unchecked_append((u8)(value >> 8));
-    m_buffer.data.unchecked_append((u8)(value >> 16));
-    m_buffer.data.unchecked_append((u8)(value >> 24));
-    m_buffer.data.unchecked_append((u8)(value >> 32));
-    m_buffer.data.unchecked_append((u8)(value >> 40));
-    m_buffer.data.unchecked_append((u8)(value >> 48));
-    m_buffer.data.unchecked_append((u8)(value >> 56));
-    return *this;
+    auto result = encoder.append(reinterpret_cast<u8 const*>(value.characters_without_null_termination()), value.length());
+    return !result.is_error();
 }
 
-Encoder& Encoder::operator<<(float value)
+template<>
+bool encode(Encoder& encoder, DeprecatedString const& value)
 {
-    u32 as_u32 = bit_cast<u32>(value);
-    return *this << as_u32;
-}
+    if (value.is_null())
+        return encoder.encode(-1);
 
-Encoder& Encoder::operator<<(double value)
-{
-    u64 as_u64 = bit_cast<u64>(value);
-    return *this << as_u64;
+    if (!encoder.encode(static_cast<i32>(value.length())))
+        return false;
+    return encoder.encode(value.view());
 }
 
-Encoder& Encoder::operator<<(char const* value)
+template<>
+bool encode(Encoder& encoder, ByteBuffer const& value)
 {
-    return *this << StringView { value, strlen(value) };
-}
+    if (!encoder.encode(static_cast<i32>(value.size())))
+        return false;
 
-Encoder& Encoder::operator<<(StringView value)
-{
-    m_buffer.data.append((u8 const*)value.characters_without_null_termination(), value.length());
-    return *this;
+    auto result = encoder.append(value.data(), value.size());
+    return !result.is_error();
 }
 
-Encoder& Encoder::operator<<(DeprecatedString const& value)
+template<>
+bool encode(Encoder& encoder, JsonValue const& value)
 {
-    if (value.is_null())
-        return *this << (i32)-1;
-    *this << static_cast<i32>(value.length());
-    return *this << value.view();
+    return encoder.encode(value.serialized<StringBuilder>());
 }
 
-Encoder& Encoder::operator<<(ByteBuffer const& value)
+template<>
+bool encode(Encoder& encoder, URL const& value)
 {
-    *this << static_cast<i32>(value.size());
-    m_buffer.data.append(value.data(), value.size());
-    return *this;
+    return encoder.encode(value.to_deprecated_string());
 }
 
-Encoder& Encoder::operator<<(JsonValue const& value)
+template<>
+bool encode(Encoder& encoder, Dictionary const& dictionary)
 {
-    *this << value.serialized<StringBuilder>();
-    return *this;
-}
+    if (!encoder.encode(static_cast<u64>(dictionary.size())))
+        return false;
 
-Encoder& Encoder::operator<<(URL const& value)
-{
-    return *this << value.to_deprecated_string();
-}
+    bool had_error = false;
 
-Encoder& Encoder::operator<<(Dictionary const& dictionary)
-{
-    *this << (u64)dictionary.size();
-    dictionary.for_each_entry([this](auto& key, auto& value) {
-        *this << key << value;
+    dictionary.for_each_entry([&](auto const& key, auto const& value) {
+        if (had_error)
+            return;
+        if (!encoder.encode(key) || !encoder.encode(value))
+            had_error = true;
     });
-    return *this;
+
+    return !had_error;
 }
 
-Encoder& Encoder::operator<<(File const& file)
+template<>
+bool encode(Encoder& encoder, File const& file)
 {
     int fd = file.fd();
+
     if (fd != -1) {
         auto result = dup(fd);
         if (result < 0) {
@@ -192,40 +103,49 @@ Encoder& Encoder::operator<<(File const& file)
         }
         fd = result;
     }
-    m_buffer.fds.append(adopt_ref(*new AutoCloseFileDescriptor(fd)));
-    return *this;
+
+    if (encoder.append_file_descriptor(fd).is_error())
+        return false;
+    return true;
 }
 
-// No-op.
-Encoder& Encoder::operator<<(AK::Empty const&)
+template<>
+bool encode(Encoder&, Empty const&)
 {
-    return *this;
+    return true;
 }
 
 template<>
 bool encode(Encoder& encoder, Core::AnonymousBuffer const& buffer)
 {
-    encoder << buffer.is_valid();
+    if (!encoder.encode(buffer.is_valid()))
+        return false;
+
     if (buffer.is_valid()) {
-        encoder << (u32)buffer.size();
-        encoder << IPC::File(buffer.fd());
+        if (!encoder.encode(static_cast<u32>(buffer.size())))
+            return false;
+        if (!encoder.encode(IPC::File { buffer.fd() }))
+            return false;
     }
+
     return true;
 }
 
 template<>
 bool encode(Encoder& encoder, Core::DateTime const& datetime)
 {
-    encoder << static_cast<i64>(datetime.timestamp());
-    return true;
+    return encoder.encode(static_cast<i64>(datetime.timestamp()));
 }
 
 template<>
 bool encode(Encoder& encoder, Core::ProxyData const& proxy)
 {
-    encoder << to_underlying(proxy.type);
-    encoder << proxy.host_ipv4;
-    encoder << proxy.port;
+    if (!encoder.encode(proxy.type))
+        return false;
+    if (!encoder.encode(proxy.host_ipv4))
+        return false;
+    if (!encoder.encode(proxy.port))
+        return false;
     return true;
 }
 

Userland/Libraries/LibIPC/Encoder.h

@@ -11,6 +11,7 @@
 #include <AK/StdLibExtras.h>
 #include <AK/Variant.h>
 #include <LibCore/SharedCircularQueue.h>
+#include <LibIPC/Concepts.h>
 #include <LibIPC/Forward.h>
 #include <LibIPC/Message.h>
 
@@ -30,100 +31,37 @@ public:
     {
     }
 
-    Encoder& operator<<(bool);
-    Encoder& operator<<(u8);
-    Encoder& operator<<(u16);
-    Encoder& operator<<(unsigned);
-    Encoder& operator<<(unsigned long);
-    Encoder& operator<<(unsigned long long);
-    Encoder& operator<<(i8);
-    Encoder& operator<<(i16);
-    Encoder& operator<<(i32);
-    Encoder& operator<<(i64);
-    Encoder& operator<<(float);
-    Encoder& operator<<(double);
-    Encoder& operator<<(char const*);
-    Encoder& operator<<(StringView);
-    Encoder& operator<<(DeprecatedString const&);
-    Encoder& operator<<(ByteBuffer const&);
-    Encoder& operator<<(JsonValue const&);
-    Encoder& operator<<(URL const&);
-    Encoder& operator<<(Dictionary const&);
-    Encoder& operator<<(File const&);
-    Encoder& operator<<(AK::Empty const&);
-    template<typename K, typename V>
-    Encoder& operator<<(HashMap<K, V> const& hashmap)
-    {
-        *this << (u32)hashmap.size();
-        for (auto it : hashmap) {
-            *this << it.key;
-            *this << it.value;
-        }
-        return *this;
-    }
-
-    template<typename K, typename V>
-    Encoder& operator<<(OrderedHashMap<K, V> const& hashmap)
+    template<typename T>
+    Encoder& operator<<(T const& value)
     {
-        *this << (u32)hashmap.size();
-        for (auto it : hashmap) {
-            *this << it.key;
-            *this << it.value;
-        }
+        encode(value);
         return *this;
     }
 
     template<typename T>
-    Encoder& operator<<(Vector<T> const& vector)
-    {
-        *this << (u64)vector.size();
-        for (auto& value : vector)
-            *this << value;
-        return *this;
-    }
+    bool encode(T const& value);
 
-    template<typename T, size_t Size>
-    Encoder& operator<<(Core::SharedSingleProducerCircularQueue<T, Size> const& queue)
+    ErrorOr<void> extend_capacity(size_t capacity)
     {
-        *this << IPC::File(queue.fd());
-        return *this;
+        return m_buffer.data.try_ensure_capacity(m_buffer.data.size() + capacity);
     }
 
-    template<typename... VariantTypes>
-    Encoder& operator<<(AK::Variant<VariantTypes...> const& variant)
+    void append(u8 value)
     {
-        *this << variant.index();
-        variant.visit([this](auto const& underlying_value) { *this << underlying_value; });
-        return *this;
+        m_buffer.data.unchecked_append(value);
     }
 
-    template<Enum T>
-    Encoder& operator<<(T const& enum_value)
+    ErrorOr<void> append(u8 const* values, size_t count)
     {
-        *this << AK::to_underlying(enum_value);
-        return *this;
+        TRY(extend_capacity(count));
+        m_buffer.data.unchecked_append(values, count);
+        return {};
     }
 
-    template<typename T>
-    Encoder& operator<<(T const& value)
+    ErrorOr<void> append_file_descriptor(int fd)
     {
-        encode(value);
-        return *this;
-    }
-
-    template<typename T>
-    Encoder& operator<<(Optional<T> const& optional)
-    {
-        *this << optional.has_value();
-        if (optional.has_value())
-            *this << optional.value();
-        return *this;
-    }
-
-    template<typename T>
-    void encode(T const& value)
-    {
-        IPC::encode(*this, value);
+        auto auto_fd = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) AutoCloseFileDescriptor(fd)));
+        return m_buffer.fds.try_append(move(auto_fd));
     }
 
 private:
@@ -133,4 +71,137 @@ private:
     MessageBuffer& m_buffer;
 };
 
+template<Arithmetic T>
+bool encode(Encoder& encoder, T const& value)
+{
+    if (encoder.extend_capacity(sizeof(T)).is_error())
+        return false;
+
+    if constexpr (sizeof(T) == 1) {
+        encoder.append(static_cast<u8>(value));
+    } else if constexpr (sizeof(T) == 2) {
+        encoder.append(static_cast<u8>(value));
+        encoder.append(static_cast<u8>(value >> 8));
+    } else if constexpr (sizeof(T) == 4) {
+        encoder.append(static_cast<u8>(value));
+        encoder.append(static_cast<u8>(value >> 8));
+        encoder.append(static_cast<u8>(value >> 16));
+        encoder.append(static_cast<u8>(value >> 24));
+    } else if constexpr (sizeof(T) == 8) {
+        encoder.append(static_cast<u8>(value));
+        encoder.append(static_cast<u8>(value >> 8));
+        encoder.append(static_cast<u8>(value >> 16));
+        encoder.append(static_cast<u8>(value >> 24));
+        encoder.append(static_cast<u8>(value >> 32));
+        encoder.append(static_cast<u8>(value >> 40));
+        encoder.append(static_cast<u8>(value >> 48));
+        encoder.append(static_cast<u8>(value >> 56));
+    } else {
+        static_assert(DependentFalse<T>);
+    }
+
+    return true;
+}
+
+template<Enum T>
+bool encode(Encoder& encoder, T const& value)
+{
+    return encoder.encode(to_underlying(value));
+}
+
+template<>
+bool encode(Encoder&, float const&);
+
+template<>
+bool encode(Encoder&, double const&);
+
+template<>
+bool encode(Encoder&, StringView const&);
+
+template<>
+bool encode(Encoder&, DeprecatedString const&);
+
+template<>
+bool encode(Encoder&, ByteBuffer const&);
+
+template<>
+bool encode(Encoder&, JsonValue const&);
+
+template<>
+bool encode(Encoder&, URL const&);
+
+template<>
+bool encode(Encoder&, Dictionary const&);
+
+template<>
+bool encode(Encoder&, File const&);
+
+template<>
+bool encode(Encoder&, Empty const&);
+
+template<Concepts::Vector T>
+bool encode(Encoder& encoder, T const& vector)
+{
+    if (!encoder.encode(static_cast<u64>(vector.size())))
+        return false;
+
+    for (auto const& value : vector) {
+        if (!encoder.encode(value))
+            return false;
+    }
+
+    return true;
+}
+
+template<Concepts::HashMap T>
+bool encode(Encoder& encoder, T const& hashmap)
+{
+    if (!encoder.encode(static_cast<u32>(hashmap.size())))
+        return false;
+
+    for (auto it : hashmap) {
+        if (!encoder.encode(it.key))
+            return false;
+        if (!encoder.encode(it.value))
+            return false;
+    }
+
+    return true;
+}
+
+template<Concepts::SharedSingleProducerCircularQueue T>
+bool encode(Encoder& encoder, T const& queue)
+{
+    return encoder.encode(IPC::File { queue.fd() });
+}
+
+template<Concepts::Optional T>
+bool encode(Encoder& encoder, T const& optional)
+{
+    if (!encoder.encode(optional.has_value()))
+        return false;
+
+    if (optional.has_value())
+        return encoder.encode(optional.value());
+    return true;
+}
+
+template<Concepts::Variant T>
+bool encode(Encoder& encoder, T const& variant)
+{
+    if (!encoder.encode(variant.index()))
+        return false;
+
+    return variant.visit([&](auto const& value) {
+        return encoder.encode(value);
+    });
+}
+
+// This must be last so that it knows about the above specializations.
+template<typename T>
+bool Encoder::encode(T const& value)
+{
+    return IPC::encode(*this, value);
+}
+
 }