From 8f2b642415a488064e214a1a6428fec422dd8374 Mon Sep 17 00:00:00 2001
From: Lectem <lectem@gmail.com>
Date: Mon, 26 Dec 2016 13:39:45 +0100
Subject: [PATCH 1/5] IPC helpers

---
 src/core/CMakeLists.txt    |   1 +
 src/core/hle/ipc.h         | 111 +++++++++-------
 src/core/hle/ipc_helpers.h | 259 +++++++++++++++++++++++++++++++++++++
 3 files changed, 323 insertions(+), 48 deletions(-)
 create mode 100644 src/core/hle/ipc_helpers.h

diff --git a/src/core/CMakeLists.txt b/src/core/CMakeLists.txt
index 3621449b3..c33555bd5 100644
--- a/src/core/CMakeLists.txt
+++ b/src/core/CMakeLists.txt
@@ -206,6 +206,7 @@ set(HEADERS
             hle/config_mem.h
             hle/function_wrappers.h
             hle/ipc.h
+            hle/ipc_helpers.h
             hle/applets/applet.h
             hle/applets/erreula.h
             hle/applets/mii_selector.h
diff --git a/src/core/hle/ipc.h b/src/core/hle/ipc.h
index 4e094faa7..4f7e5d050 100644
--- a/src/core/hle/ipc.h
+++ b/src/core/hle/ipc.h
@@ -18,12 +18,26 @@ static const int kCommandHeaderOffset = 0x80; ///< Offset into command buffer of
  * the thread's TLS to an intermediate buffer in kernel memory, and then copied again to
  * the service handler process' memory.
  * @param offset Optional offset into command buffer
+ * @param offset Optional offset into command buffer (in bytes)
  * @return Pointer to command buffer
  */
 inline u32* GetCommandBuffer(const int offset = 0) {
     return (u32*)Memory::GetPointer(GetCurrentThread()->GetTLSAddress() + kCommandHeaderOffset +
                                     offset);
 }
+
+static const int kStaticBuffersOffset =
+    0x100; ///< Offset into static buffers, relative to command buffer header
+
+/**
+ * Returns a pointer to the static buffers area in the current thread's TLS
+ * TODO(Subv): cf. GetCommandBuffer
+ * @param offset Optional offset into static buffers area (in bytes)
+ * @return Pointer to static buffers area
+ */
+inline u32* GetStaticBuffers(const int offset = 0) {
+    return GetCommandBuffer(kStaticBuffersOffset + offset);
+}
 }
 
 namespace IPC {
@@ -40,35 +54,34 @@ enum DescriptorType : u32 {
     CallingPid = 0x20,
 };
 
-/**
- * @brief Creates a command header to be used for IPC
- * @param command_id            ID of the command to create a header for.
- * @param normal_params         Size of the normal parameters in words. Up to 63.
- * @param translate_params_size Size of the translate parameters in words. Up to 63.
- * @return The created IPC header.
- *
- * Normal parameters are sent directly to the process while the translate parameters might go
- * through modifications and checks by the kernel.
- * The translate parameters are described by headers generated with the IPC::*Desc functions.
- *
- * @note While #normal_params is equivalent to the number of normal parameters,
- * #translate_params_size includes the size occupied by the translate parameters headers.
- */
-constexpr u32 MakeHeader(u16 command_id, unsigned int normal_params,
-                         unsigned int translate_params_size) {
-    return (u32(command_id) << 16) | ((u32(normal_params) & 0x3F) << 6) |
-           (u32(translate_params_size) & 0x3F);
-}
-
 union Header {
     u32 raw;
     BitField<0, 6, u32> translate_params_size;
-    BitField<6, 6, u32> normal_params;
+    BitField<6, 6, u32> normal_params_size;
     BitField<16, 16, u32> command_id;
 };
 
-inline Header ParseHeader(u32 header) {
-    return {header};
+/**
+* @brief Creates a command header to be used for IPC
+* @param command_id            ID of the command to create a header for.
+* @param normal_params_size         Size of the normal parameters in words. Up to 63.
+* @param translate_params_size Size of the translate parameters in words. Up to 63.
+* @return The created IPC header.
+*
+* Normal parameters are sent directly to the process while the translate parameters might go
+* through modifications and checks by the kernel.
+* The translate parameters are described by headers generated with the IPC::*Desc functions.
+*
+* @note While #normal_params_size is equivalent to the number of normal parameters,
+* #translate_params_size includes the size occupied by the translate parameters headers.
+*/
+inline u32 MakeHeader(u16 command_id, unsigned int normal_params_size,
+                      unsigned int translate_params_size) {
+    Header header;
+    header.command_id.Assign(command_id);
+    header.normal_params_size.Assign(normal_params_size);
+    header.translate_params_size.Assign(translate_params_size);
+    return header.raw;
 }
 
 constexpr u32 MoveHandleDesc(u32 num_handles = 1) {
@@ -83,7 +96,7 @@ constexpr u32 CallingPidDesc() {
     return CallingPid;
 }
 
-constexpr bool isHandleDescriptor(u32 descriptor) {
+constexpr bool IsHandleDescriptor(u32 descriptor) {
     return (descriptor & 0xF) == 0x0;
 }
 
@@ -91,30 +104,31 @@ constexpr u32 HandleNumberFromDesc(u32 handle_descriptor) {
     return (handle_descriptor >> 26) + 1;
 }
 
-constexpr u32 StaticBufferDesc(u32 size, u8 buffer_id) {
-    return StaticBuffer | (size << 14) | ((buffer_id & 0xF) << 10);
-}
-
 union StaticBufferDescInfo {
     u32 raw;
+    BitField<0, 4, u32> descriptor_type;
     BitField<10, 4, u32> buffer_id;
     BitField<14, 18, u32> size;
 };
 
-inline StaticBufferDescInfo ParseStaticBufferDesc(const u32 desc) {
-    return {desc};
+inline u32 StaticBufferDesc(u32 size, u8 buffer_id) {
+    StaticBufferDescInfo info;
+    info.descriptor_type.Assign(StaticBuffer);
+    info.buffer_id.Assign(buffer_id);
+    info.size.Assign(size);
+    return info.raw;
 }
 
 /**
- * @brief Creates a header describing a buffer to be sent over PXI.
- * @param size         Size of the buffer. Max 0x00FFFFFF.
- * @param buffer_id    The Id of the buffer. Max 0xF.
- * @param is_read_only true if the buffer is read-only. If false, the buffer is considered to have
- * read-write access.
- * @return The created PXI buffer header.
- *
- * The next value is a phys-address of a table located in the BASE memregion.
- */
+* @brief Creates a header describing a buffer to be sent over PXI.
+* @param size         Size of the buffer. Max 0x00FFFFFF.
+* @param buffer_id    The Id of the buffer. Max 0xF.
+* @param is_read_only true if the buffer is read-only. If false, the buffer is considered to have
+* read-write access.
+* @return The created PXI buffer header.
+*
+* The next value is a phys-address of a table located in the BASE memregion.
+*/
 inline u32 PXIBufferDesc(u32 size, unsigned buffer_id, bool is_read_only) {
     u32 type = PXIBuffer;
     if (is_read_only)
@@ -122,29 +136,30 @@ inline u32 PXIBufferDesc(u32 size, unsigned buffer_id, bool is_read_only) {
     return type | (size << 8) | ((buffer_id & 0xF) << 4);
 }
 
-enum MappedBufferPermissions {
+enum MappedBufferPermissions : u32 {
     R = 1,
     W = 2,
     RW = R | W,
 };
 
-constexpr u32 MappedBufferDesc(u32 size, MappedBufferPermissions perms) {
-    return MappedBuffer | (size << 4) | (u32(perms) << 1);
-}
-
 union MappedBufferDescInfo {
     u32 raw;
-    BitField<4, 28, u32> size;
+    BitField<0, 4, u32> flags;
     BitField<1, 2, MappedBufferPermissions> perms;
+    BitField<4, 28, u32> size;
 };
 
-inline MappedBufferDescInfo ParseMappedBufferDesc(const u32 desc) {
-    return {desc};
+inline u32 MappedBufferDesc(u32 size, MappedBufferPermissions perms) {
+    MappedBufferDescInfo info;
+    info.flags.Assign(MappedBuffer);
+    info.perms.Assign(perms);
+    info.size.Assign(size);
+    return info.raw;
 }
 
 inline DescriptorType GetDescriptorType(u32 descriptor) {
     // Note: Those checks must be done in this order
-    if (isHandleDescriptor(descriptor))
+    if (IsHandleDescriptor(descriptor))
         return (DescriptorType)(descriptor & 0x30);
 
     // handle the fact that the following descriptors can have rights
diff --git a/src/core/hle/ipc_helpers.h b/src/core/hle/ipc_helpers.h
new file mode 100644
index 000000000..6089c39c7
--- /dev/null
+++ b/src/core/hle/ipc_helpers.h
@@ -0,0 +1,259 @@
+// Copyright 2016 Citra Emulator Project
+// Licensed under GPLv2 or any later version
+// Refer to the license.txt file included.
+
+#pragma once
+#include "core/hle/ipc.h"
+#include "core/hle/kernel/kernel.h"
+
+namespace IPC {
+
+class RequestHelperBase {
+protected:
+    u32* cmdbuf;
+    ptrdiff_t index = 1;
+    Header header;
+
+public:
+    RequestHelperBase(u32* command_buffer, Header command_header)
+        : cmdbuf(command_buffer), header(command_header) {}
+
+    /// Returns the total size of the request in words
+    size_t TotalSize() const {
+        return 1 /* command header */ + header.normal_params_size + header.translate_params_size;
+    }
+
+    void ValidateHeader() {
+        DEBUG_ASSERT_MSG(index == TotalSize(), "Operations do not match the header (cmd 0x%x)",
+                         header.raw);
+    }
+};
+
+class RequestBuilder : public RequestHelperBase {
+public:
+    RequestBuilder(u32* command_buffer, Header command_header)
+        : RequestHelperBase(command_buffer, command_header) {
+        cmdbuf[0] = header.raw;
+    }
+    explicit RequestBuilder(u32* command_buffer, u32 command_header)
+        : RequestBuilder(command_buffer, Header{command_header}) {}
+    RequestBuilder(u32* command_buffer, u16 command_id, unsigned normal_params_size,
+                   unsigned translate_params_size)
+        : RequestBuilder(command_buffer,
+                         MakeHeader(command_id, normal_params_size, translate_params_size)) {}
+
+    // Validate on destruction, as there shouldn't be any case where we don't want it
+    ~RequestBuilder() {
+        ValidateHeader();
+    }
+
+    template <typename T>
+    void Push(T value);
+
+    template <>
+    void Push(u32);
+
+    template <typename First, class... Other>
+    void Push(First first_value, const Other&... other_values) {
+        Push(first_value);
+        Push(other_values...);
+    }
+
+    /**
+     * @brief Copies the content of the given trivially copyable class to the buffer as a normal
+     * param
+     * @note: The input class must be correctly packed/padded to fit hardware layout.
+     */
+    template <typename T>
+    void PushRaw(const T& value) {
+        static_assert(std::is_trivially_copyable<T>(), "Raw types should be trivially copyable");
+        std::memcpy(cmdbuf + index, &value, sizeof(T));
+        index += (sizeof(T) + 3) / 4; // round up to word length
+    }
+
+    // TODO : ensure that translate params are added after all regular params
+    template <typename... H>
+    void PushCopyHandles(H... handles) {
+        Push(CopyHandleDesc(sizeof...(H)));
+        Push(static_cast<Kernel::Handle>(handles)...);
+    }
+
+    template <typename... H>
+    void PushMoveHandles(H... handles) {
+        Push(MoveHandleDesc(sizeof...(H)));
+        Push(static_cast<Kernel::Handle>(handles)...);
+    }
+
+    void PushCurrentPIDHandle() {
+        Push(CallingPidDesc());
+        Push<u32>(0);
+    }
+
+    void PushStaticBuffer(VAddr buffer_vaddr, u32 size, u8 buffer_id) {
+        Push(StaticBufferDesc(size, buffer_id));
+        Push(buffer_vaddr);
+    }
+
+    void PushMappedBuffer(VAddr buffer_vaddr, u32 size, MappedBufferPermissions perms) {
+        Push(MappedBufferDesc(size, perms));
+        Push(buffer_vaddr);
+    }
+};
+
+class RequestParser : public RequestHelperBase {
+public:
+    RequestParser(u32* command_buffer, Header command_header)
+        : RequestHelperBase(command_buffer, command_header) {}
+    explicit RequestParser(u32* command_buffer, u32 command_header)
+        : RequestParser(command_buffer, Header{command_header}) {}
+    RequestParser(u32* command_buffer, u16 command_id, unsigned normal_params_size,
+                  unsigned translate_params_size)
+        : RequestParser(command_buffer,
+                        MakeHeader(command_id, normal_params_size, translate_params_size)) {}
+
+    RequestBuilder MakeBuilder(u32 normal_params_size, u32 translate_params_size,
+                               bool validateHeader = true) {
+        if (validateHeader)
+            ValidateHeader();
+        Header builderHeader{
+            MakeHeader(header.command_id, normal_params_size, translate_params_size)};
+        return {cmdbuf, builderHeader};
+    }
+
+    template <typename T>
+    T Pop();
+
+    template <>
+    u32 Pop<u32>();
+
+    template <typename T>
+    void Pop(T& value) {
+        value = Pop<T>();
+    }
+
+    template <typename First, class... Other>
+    void Pop(First& first_value, Other&... other_values) {
+        first_value = Pop<First>();
+        Pop(other_values...);
+    }
+
+    Kernel::Handle PopHandle() {
+        const u32 handle_descriptor = Pop<u32>();
+        DEBUG_ASSERT_MSG(IsHandleDescriptor(handle_descriptor),
+                         "Tried to pop handle(s) but the descriptor is not a handle descriptor");
+        DEBUG_ASSERT_MSG(HandleNumberFromDesc(handle_descriptor) == 1,
+                         "Descriptor indicates that there isn't exactly one handle");
+        return Pop<Kernel::Handle>();
+    }
+
+    template <typename... H>
+    void PopHandles(H&... handles) {
+        const u32 handle_descriptor = Pop<u32>();
+        const int handles_number = sizeof...(H);
+        DEBUG_ASSERT_MSG(IsHandleDescriptor(handle_descriptor),
+                         "Tried to pop handle(s) but the descriptor is not a handle descriptor");
+        DEBUG_ASSERT_MSG(handles_number == HandleNumberFromDesc(handle_descriptor),
+                         "Number of handles doesn't match the descriptor");
+        Pop(static_cast<Kernel::Handle&>(handles)...);
+    }
+
+    /**
+     * @brief Pops the static buffer vaddr
+     * @return                  The virtual address of the buffer
+     * @param[out] data_size    If non-null, the pointed value will be set to the size of the data
+     * @param[out] useStaticBuffersToGetVaddr Indicates if we should read the vaddr from the static
+     * buffers (which is the correct thing to do, but no service presently implement it) instead of
+     * using the same value as the process who sent the request
+     * given by the source process
+     *
+     * Static buffers must be set up before any IPC request using those is sent.
+     * It is the duty of the process (usually services) to allocate and set up the receiving static
+     * buffer information
+     * Please note that the setup uses virtual addresses.
+     */
+    VAddr PopStaticBuffer(size_t* data_size = nullptr, bool useStaticBuffersToGetVaddr = false) {
+        const u32 sbuffer_descriptor = Pop<u32>();
+        StaticBufferDescInfo bufferInfo{sbuffer_descriptor};
+        if (data_size != nullptr)
+            *data_size = bufferInfo.size;
+        if (!useStaticBuffersToGetVaddr)
+            return Pop<VAddr>();
+        else {
+            ASSERT_MSG(0, "remove the assert if multiprocess/IPC translation are implemented.");
+            // The buffer has already been copied to the static buffer by the kernel during
+            // translation
+            Pop<VAddr>(); // Pop the calling process buffer address
+                          // and get the vaddr from the static buffers
+            return cmdbuf[(0x100 >> 2) + bufferInfo.buffer_id * 2 + 1];
+        }
+    }
+
+    /**
+     * @brief Pops the mapped buffer vaddr
+     * @return                  The virtual address of the buffer
+     * @param[out] data_size    If non-null, the pointed value will be set to the size of the data
+     * given by the source process
+     * @param[out] buffer_perms If non-null, the pointed value will be set to the permissions of the
+     * buffer
+     */
+    VAddr PopMappedBuffer(size_t* data_size = nullptr,
+                          MappedBufferPermissions* buffer_perms = nullptr) {
+        const u32 sbuffer_descriptor = Pop<u32>();
+        MappedBufferDescInfo bufferInfo{sbuffer_descriptor};
+        if (data_size != nullptr)
+            *data_size = bufferInfo.size;
+        if (buffer_perms != nullptr)
+            *buffer_perms = bufferInfo.perms;
+        return Pop<VAddr>();
+    }
+
+    /**
+     * @brief Reads the next normal parameters as a struct, by copying it
+     * @note: The output class must be correctly packed/padded to fit hardware layout.
+     */
+    template <typename T>
+    void PopRaw(T& value) {
+        static_assert(std::is_trivially_copyable<T>(), "Raw types should be trivially copyable");
+        std::memcpy(&value, cmdbuf + index, sizeof(T));
+        index += (sizeof(T) + 3) / 4; // round up to word length
+    }
+};
+
+/// Pop ///
+
+template <>
+inline u32 RequestParser::Pop<u32>() {
+    return cmdbuf[index++];
+}
+
+template <>
+inline u64 RequestParser::Pop<u64>() {
+    const u64 lsw = Pop<u32>();
+    const u64 msw = Pop<u32>();
+    return msw << 32 | lsw;
+}
+
+template <>
+inline ResultCode RequestParser::Pop<ResultCode>() {
+    return ResultCode{Pop<u32>()};
+}
+
+/// Push ///
+
+template <>
+inline void RequestBuilder::Push<u32>(u32 value) {
+    cmdbuf[index++] = value;
+}
+
+template <>
+inline void RequestBuilder::Push<u64>(u64 value) {
+    Push(static_cast<u32>(value));
+    Push(static_cast<u32>(value >> 32));
+}
+
+template <>
+inline void RequestBuilder::Push<ResultCode>(ResultCode value) {
+    Push(value.raw);
+}
+
+} // namespace IPC

From 8baae9d9828eff9bcaf24c5767ed03d05f7e617b Mon Sep 17 00:00:00 2001
From: Lectem <lectem@gmail.com>
Date: Mon, 26 Dec 2016 13:58:50 +0100
Subject: [PATCH 2/5] IPC helpers example

---
 src/core/hle/service/fs/fs_user.cpp | 27 +++++++++--------
 src/core/hle/service/service.h      |  1 +
 src/core/hle/service/y2r_u.cpp      | 47 +++++++++++++++--------------
 3 files changed, 40 insertions(+), 35 deletions(-)

diff --git a/src/core/hle/service/fs/fs_user.cpp b/src/core/hle/service/fs/fs_user.cpp
index 337da1387..33b290699 100644
--- a/src/core/hle/service/fs/fs_user.cpp
+++ b/src/core/hle/service/fs/fs_user.cpp
@@ -54,15 +54,17 @@ static void Initialize(Service::Interface* self) {
  *      3 : File handle
  */
 static void OpenFile(Service::Interface* self) {
-    u32* cmd_buff = Kernel::GetCommandBuffer();
+    // The helper should be passed by argument to the function
+    IPC::RequestParser rp(Kernel::GetCommandBuffer(), {0x080201C2});
+    rp.Pop<u32>(); // Always 0 ?
 
-    ArchiveHandle archive_handle = MakeArchiveHandle(cmd_buff[2], cmd_buff[3]);
-    auto filename_type = static_cast<FileSys::LowPathType>(cmd_buff[4]);
-    u32 filename_size = cmd_buff[5];
+    ArchiveHandle archive_handle = rp.Pop<u64>();
+    auto filename_type = static_cast<FileSys::LowPathType>(rp.Pop<u32>());
+    u32 filename_size = rp.Pop<u32>();
     FileSys::Mode mode;
-    mode.hex = cmd_buff[6];
-    u32 attributes = cmd_buff[7]; // TODO(Link Mauve): do something with those attributes.
-    u32 filename_ptr = cmd_buff[9];
+    mode.hex = rp.Pop<u32>();
+    u32 attributes = rp.Pop<u32>(); // TODO(Link Mauve): do something with those attributes.
+    VAddr filename_ptr = rp.PopStaticBuffer();
     FileSys::Path file_path(filename_type, filename_size, filename_ptr);
 
     LOG_DEBUG(Service_FS, "path=%s, mode=%u attrs=%u", file_path.DebugStr().c_str(), mode.hex,
@@ -70,16 +72,17 @@ static void OpenFile(Service::Interface* self) {
 
     ResultVal<std::shared_ptr<File>> file_res =
         OpenFileFromArchive(archive_handle, file_path, mode);
-    cmd_buff[1] = file_res.Code().raw;
+    IPC::RequestBuilder rb = rp.MakeBuilder(1, 2);
+    rb.Push(file_res.Code());
     if (file_res.Succeeded()) {
         std::shared_ptr<File> file = *file_res;
         auto sessions = ServerSession::CreateSessionPair(file->GetName(), file);
         file->ClientConnected(std::get<Kernel::SharedPtr<Kernel::ServerSession>>(sessions));
-        cmd_buff[3] = Kernel::g_handle_table
-                          .Create(std::get<Kernel::SharedPtr<Kernel::ClientSession>>(sessions))
-                          .MoveFrom();
+        rb.PushMoveHandles(Kernel::g_handle_table
+                               .Create(std::get<Kernel::SharedPtr<Kernel::ClientSession>>(sessions))
+                               .MoveFrom());
     } else {
-        cmd_buff[3] = 0;
+        rb.PushMoveHandles(0);
         LOG_ERROR(Service_FS, "failed to get a handle for file %s", file_path.DebugStr().c_str());
     }
 }
diff --git a/src/core/hle/service/service.h b/src/core/hle/service/service.h
index a7ba7688f..e6a5f1417 100644
--- a/src/core/hle/service/service.h
+++ b/src/core/hle/service/service.h
@@ -10,6 +10,7 @@
 #include <boost/container/flat_map.hpp>
 #include "common/common_types.h"
 #include "core/hle/ipc.h"
+#include "core/hle/ipc_helpers.h"
 #include "core/hle/kernel/client_port.h"
 #include "core/hle/kernel/thread.h"
 #include "core/hle/result.h"
diff --git a/src/core/hle/service/y2r_u.cpp b/src/core/hle/service/y2r_u.cpp
index a20194107..35eb2b597 100644
--- a/src/core/hle/service/y2r_u.cpp
+++ b/src/core/hle/service/y2r_u.cpp
@@ -281,37 +281,39 @@ static void GetTransferEndEvent(Interface* self) {
 }
 
 static void SetSendingY(Interface* self) {
-    u32* cmd_buff = Kernel::GetCommandBuffer();
+    // The helper should be passed by argument to the function
+    IPC::RequestParser rp(Kernel::GetCommandBuffer(), 0x00100102);
+    conversion.src_Y.address = rp.Pop<u32>();
+    conversion.src_Y.image_size = rp.Pop<u32>();
+    conversion.src_Y.transfer_unit = rp.Pop<u32>();
+    conversion.src_Y.gap = rp.Pop<u32>();
+    Kernel::Handle src_process_handle = rp.PopHandle();
 
-    conversion.src_Y.address = cmd_buff[1];
-    conversion.src_Y.image_size = cmd_buff[2];
-    conversion.src_Y.transfer_unit = cmd_buff[3];
-    conversion.src_Y.gap = cmd_buff[4];
-
-    cmd_buff[0] = IPC::MakeHeader(0x10, 1, 0);
-    cmd_buff[1] = RESULT_SUCCESS.raw;
+    IPC::RequestBuilder rb = rp.MakeBuilder(1, 0);
+    rb.Push(RESULT_SUCCESS);
 
     LOG_DEBUG(Service_Y2R, "called image_size=0x%08X, transfer_unit=%hu, transfer_stride=%hu, "
                            "src_process_handle=0x%08X",
               conversion.src_Y.image_size, conversion.src_Y.transfer_unit, conversion.src_Y.gap,
-              cmd_buff[6]);
+              src_process_handle);
 }
 
 static void SetSendingU(Interface* self) {
-    u32* cmd_buff = Kernel::GetCommandBuffer();
+    // The helper should be passed by argument to the function
+    IPC::RequestParser rp(Kernel::GetCommandBuffer(), 0x00110102);
+    conversion.src_U.address = rp.Pop<u32>();
+    conversion.src_U.image_size = rp.Pop<u32>();
+    conversion.src_U.transfer_unit = rp.Pop<u32>();
+    conversion.src_U.gap = rp.Pop<u32>();
+    Kernel::Handle src_process_handle = rp.PopHandle();
 
-    conversion.src_U.address = cmd_buff[1];
-    conversion.src_U.image_size = cmd_buff[2];
-    conversion.src_U.transfer_unit = cmd_buff[3];
-    conversion.src_U.gap = cmd_buff[4];
-
-    cmd_buff[0] = IPC::MakeHeader(0x11, 1, 0);
-    cmd_buff[1] = RESULT_SUCCESS.raw;
+    IPC::RequestBuilder rb = rp.MakeBuilder(1, 0);
+    rb.Push(RESULT_SUCCESS);
 
     LOG_DEBUG(Service_Y2R, "called image_size=0x%08X, transfer_unit=%hu, transfer_stride=%hu, "
                            "src_process_handle=0x%08X",
               conversion.src_U.image_size, conversion.src_U.transfer_unit, conversion.src_U.gap,
-              cmd_buff[6]);
+              src_process_handle);
 }
 
 static void SetSendingV(Interface* self) {
@@ -559,11 +561,10 @@ static void GetAlpha(Interface* self) {
 }
 
 static void SetDitheringWeightParams(Interface* self) {
-    u32* cmd_buff = Kernel::GetCommandBuffer();
-    std::memcpy(&dithering_weight_params, &cmd_buff[1], sizeof(DitheringWeightParams));
-
-    cmd_buff[0] = IPC::MakeHeader(0x24, 1, 0);
-    cmd_buff[1] = RESULT_SUCCESS.raw;
+    IPC::RequestParser rp(Kernel::GetCommandBuffer(), 0x24, 8, 0); // 0x240200
+    rp.PopRaw(dithering_weight_params);
+    IPC::RequestBuilder rb = rp.MakeBuilder(1, 0);
+    rb.Push(RESULT_SUCCESS);
 
     LOG_DEBUG(Service_Y2R, "called");
 }

From f91c51467a58b66a2ca2ae78a67f9ca076299c2e Mon Sep 17 00:00:00 2001
From: Lectem <lectem@gmail.com>
Date: Mon, 26 Dec 2016 14:42:06 +0100
Subject: [PATCH 3/5] move Pop methods out of class body

---
 src/core/hle/ipc_helpers.h | 162 ++++++++++++++++++++-----------------
 1 file changed, 89 insertions(+), 73 deletions(-)

diff --git a/src/core/hle/ipc_helpers.h b/src/core/hle/ipc_helpers.h
index 6089c39c7..68508caba 100644
--- a/src/core/hle/ipc_helpers.h
+++ b/src/core/hle/ipc_helpers.h
@@ -50,9 +50,9 @@ public:
     template <typename T>
     void Push(T value);
 
-    template <>
-    void Push(u32);
-
+    void Push(u32 value) {
+        cmdbuf[index++] = value;
+    }
     template <typename First, class... Other>
     void Push(First first_value, const Other&... other_values) {
         Push(first_value);
@@ -86,7 +86,7 @@ public:
 
     void PushCurrentPIDHandle() {
         Push(CallingPidDesc());
-        Push<u32>(0);
+        Push(u32(0));
     }
 
     void PushStaticBuffer(VAddr buffer_vaddr, u32 size, u8 buffer_id) {
@@ -100,6 +100,24 @@ public:
     }
 };
 
+/// Push ///
+
+template <>
+inline void RequestBuilder::Push<u32>(u32 value) {
+    Push(value);
+}
+
+template <>
+inline void RequestBuilder::Push<u64>(u64 value) {
+    Push(static_cast<u32>(value));
+    Push(static_cast<u32>(value >> 32));
+}
+
+template <>
+inline void RequestBuilder::Push<ResultCode>(ResultCode value) {
+    Push(value.raw);
+}
+
 class RequestParser : public RequestHelperBase {
 public:
     RequestParser(u32* command_buffer, Header command_header)
@@ -123,39 +141,16 @@ public:
     template <typename T>
     T Pop();
 
-    template <>
-    u32 Pop<u32>();
-
     template <typename T>
-    void Pop(T& value) {
-        value = Pop<T>();
-    }
+    void Pop(T& value);
 
     template <typename First, class... Other>
-    void Pop(First& first_value, Other&... other_values) {
-        first_value = Pop<First>();
-        Pop(other_values...);
-    }
+    void Pop(First& first_value, Other&... other_values);
 
-    Kernel::Handle PopHandle() {
-        const u32 handle_descriptor = Pop<u32>();
-        DEBUG_ASSERT_MSG(IsHandleDescriptor(handle_descriptor),
-                         "Tried to pop handle(s) but the descriptor is not a handle descriptor");
-        DEBUG_ASSERT_MSG(HandleNumberFromDesc(handle_descriptor) == 1,
-                         "Descriptor indicates that there isn't exactly one handle");
-        return Pop<Kernel::Handle>();
-    }
+    Kernel::Handle PopHandle();
 
     template <typename... H>
-    void PopHandles(H&... handles) {
-        const u32 handle_descriptor = Pop<u32>();
-        const int handles_number = sizeof...(H);
-        DEBUG_ASSERT_MSG(IsHandleDescriptor(handle_descriptor),
-                         "Tried to pop handle(s) but the descriptor is not a handle descriptor");
-        DEBUG_ASSERT_MSG(handles_number == HandleNumberFromDesc(handle_descriptor),
-                         "Number of handles doesn't match the descriptor");
-        Pop(static_cast<Kernel::Handle&>(handles)...);
-    }
+    void PopHandles(H&... handles);
 
     /**
      * @brief Pops the static buffer vaddr
@@ -171,22 +166,7 @@ public:
      * buffer information
      * Please note that the setup uses virtual addresses.
      */
-    VAddr PopStaticBuffer(size_t* data_size = nullptr, bool useStaticBuffersToGetVaddr = false) {
-        const u32 sbuffer_descriptor = Pop<u32>();
-        StaticBufferDescInfo bufferInfo{sbuffer_descriptor};
-        if (data_size != nullptr)
-            *data_size = bufferInfo.size;
-        if (!useStaticBuffersToGetVaddr)
-            return Pop<VAddr>();
-        else {
-            ASSERT_MSG(0, "remove the assert if multiprocess/IPC translation are implemented.");
-            // The buffer has already been copied to the static buffer by the kernel during
-            // translation
-            Pop<VAddr>(); // Pop the calling process buffer address
-                          // and get the vaddr from the static buffers
-            return cmdbuf[(0x100 >> 2) + bufferInfo.buffer_id * 2 + 1];
-        }
-    }
+    VAddr PopStaticBuffer(size_t* data_size = nullptr, bool useStaticBuffersToGetVaddr = false);
 
     /**
      * @brief Pops the mapped buffer vaddr
@@ -197,26 +177,14 @@ public:
      * buffer
      */
     VAddr PopMappedBuffer(size_t* data_size = nullptr,
-                          MappedBufferPermissions* buffer_perms = nullptr) {
-        const u32 sbuffer_descriptor = Pop<u32>();
-        MappedBufferDescInfo bufferInfo{sbuffer_descriptor};
-        if (data_size != nullptr)
-            *data_size = bufferInfo.size;
-        if (buffer_perms != nullptr)
-            *buffer_perms = bufferInfo.perms;
-        return Pop<VAddr>();
-    }
+                          MappedBufferPermissions* buffer_perms = nullptr);
 
     /**
      * @brief Reads the next normal parameters as a struct, by copying it
      * @note: The output class must be correctly packed/padded to fit hardware layout.
      */
     template <typename T>
-    void PopRaw(T& value) {
-        static_assert(std::is_trivially_copyable<T>(), "Raw types should be trivially copyable");
-        std::memcpy(&value, cmdbuf + index, sizeof(T));
-        index += (sizeof(T) + 3) / 4; // round up to word length
-    }
+    void PopRaw(T& value);
 };
 
 /// Pop ///
@@ -238,22 +206,70 @@ inline ResultCode RequestParser::Pop<ResultCode>() {
     return ResultCode{Pop<u32>()};
 }
 
-/// Push ///
-
-template <>
-inline void RequestBuilder::Push<u32>(u32 value) {
-    cmdbuf[index++] = value;
+template <typename T>
+void RequestParser::Pop(T& value) {
+    value = Pop<T>();
 }
 
-template <>
-inline void RequestBuilder::Push<u64>(u64 value) {
-    Push(static_cast<u32>(value));
-    Push(static_cast<u32>(value >> 32));
+template <typename First, class... Other>
+void RequestParser::Pop(First& first_value, Other&... other_values) {
+    first_value = Pop<First>();
+    Pop(other_values...);
 }
 
-template <>
-inline void RequestBuilder::Push<ResultCode>(ResultCode value) {
-    Push(value.raw);
+inline Kernel::Handle RequestParser::PopHandle() {
+    const u32 handle_descriptor = Pop<u32>();
+    DEBUG_ASSERT_MSG(IsHandleDescriptor(handle_descriptor),
+                     "Tried to pop handle(s) but the descriptor is not a handle descriptor");
+    DEBUG_ASSERT_MSG(HandleNumberFromDesc(handle_descriptor) == 1,
+                     "Descriptor indicates that there isn't exactly one handle");
+    return Pop<Kernel::Handle>();
+}
+
+template <typename... H>
+void RequestParser::PopHandles(H&... handles) {
+    const u32 handle_descriptor = Pop<u32>();
+    const int handles_number = sizeof...(H);
+    DEBUG_ASSERT_MSG(IsHandleDescriptor(handle_descriptor),
+                     "Tried to pop handle(s) but the descriptor is not a handle descriptor");
+    DEBUG_ASSERT_MSG(handles_number == HandleNumberFromDesc(handle_descriptor),
+                     "Number of handles doesn't match the descriptor");
+    Pop(static_cast<Kernel::Handle&>(handles)...);
+}
+
+inline VAddr RequestParser::PopStaticBuffer(size_t* data_size, bool useStaticBuffersToGetVaddr) {
+    const u32 sbuffer_descriptor = Pop<u32>();
+    StaticBufferDescInfo bufferInfo{sbuffer_descriptor};
+    if (data_size != nullptr)
+        *data_size = bufferInfo.size;
+    if (!useStaticBuffersToGetVaddr)
+        return Pop<VAddr>();
+    else {
+        ASSERT_MSG(0, "remove the assert if multiprocess/IPC translation are implemented.");
+        // The buffer has already been copied to the static buffer by the kernel during
+        // translation
+        Pop<VAddr>(); // Pop the calling process buffer address
+                      // and get the vaddr from the static buffers
+        return cmdbuf[(0x100 >> 2) + bufferInfo.buffer_id * 2 + 1];
+    }
+}
+
+inline VAddr RequestParser::PopMappedBuffer(size_t* data_size,
+                                            MappedBufferPermissions* buffer_perms) {
+    const u32 sbuffer_descriptor = Pop<u32>();
+    MappedBufferDescInfo bufferInfo{sbuffer_descriptor};
+    if (data_size != nullptr)
+        *data_size = bufferInfo.size;
+    if (buffer_perms != nullptr)
+        *buffer_perms = bufferInfo.perms;
+    return Pop<VAddr>();
+}
+
+template <typename T>
+void RequestParser::PopRaw(T& value) {
+    static_assert(std::is_trivially_copyable<T>(), "Raw types should be trivially copyable");
+    std::memcpy(&value, cmdbuf + index, sizeof(T));
+    index += (sizeof(T) + 3) / 4; // round up to word length
 }
 
 } // namespace IPC

From ee6e88fdb592b442bf433acb6dbbc2d48568d2a3 Mon Sep 17 00:00:00 2001
From: Lectem <lectem@gmail.com>
Date: Fri, 30 Dec 2016 15:55:42 +0100
Subject: [PATCH 4/5] fix comments alignment

---
 src/core/hle/ipc.h | 44 ++++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/src/core/hle/ipc.h b/src/core/hle/ipc.h
index 4f7e5d050..4535b61c0 100644
--- a/src/core/hle/ipc.h
+++ b/src/core/hle/ipc.h
@@ -62,19 +62,19 @@ union Header {
 };
 
 /**
-* @brief Creates a command header to be used for IPC
-* @param command_id            ID of the command to create a header for.
-* @param normal_params_size         Size of the normal parameters in words. Up to 63.
-* @param translate_params_size Size of the translate parameters in words. Up to 63.
-* @return The created IPC header.
-*
-* Normal parameters are sent directly to the process while the translate parameters might go
-* through modifications and checks by the kernel.
-* The translate parameters are described by headers generated with the IPC::*Desc functions.
-*
-* @note While #normal_params_size is equivalent to the number of normal parameters,
-* #translate_params_size includes the size occupied by the translate parameters headers.
-*/
+ * @brief Creates a command header to be used for IPC
+ * @param command_id            ID of the command to create a header for.
+ * @param normal_params_size         Size of the normal parameters in words. Up to 63.
+ * @param translate_params_size Size of the translate parameters in words. Up to 63.
+ * @return The created IPC header.
+ *
+ * Normal parameters are sent directly to the process while the translate parameters might go
+ * through modifications and checks by the kernel.
+ * The translate parameters are described by headers generated with the IPC::*Desc functions.
+ *
+ * @note While #normal_params_size is equivalent to the number of normal parameters,
+ * #translate_params_size includes the size occupied by the translate parameters headers.
+ */
 inline u32 MakeHeader(u16 command_id, unsigned int normal_params_size,
                       unsigned int translate_params_size) {
     Header header;
@@ -120,15 +120,15 @@ inline u32 StaticBufferDesc(u32 size, u8 buffer_id) {
 }
 
 /**
-* @brief Creates a header describing a buffer to be sent over PXI.
-* @param size         Size of the buffer. Max 0x00FFFFFF.
-* @param buffer_id    The Id of the buffer. Max 0xF.
-* @param is_read_only true if the buffer is read-only. If false, the buffer is considered to have
-* read-write access.
-* @return The created PXI buffer header.
-*
-* The next value is a phys-address of a table located in the BASE memregion.
-*/
+ * @brief Creates a header describing a buffer to be sent over PXI.
+ * @param size         Size of the buffer. Max 0x00FFFFFF.
+ * @param buffer_id    The Id of the buffer. Max 0xF.
+ * @param is_read_only true if the buffer is read-only. If false, the buffer is considered to have
+ * read-write access.
+ * @return The created PXI buffer header.
+ *
+ * The next value is a phys-address of a table located in the BASE memregion.
+ */
 inline u32 PXIBufferDesc(u32 size, unsigned buffer_id, bool is_read_only) {
     u32 type = PXIBuffer;
     if (is_read_only)

From 2ee472b9c7123fe8b70a14311312e268635b3700 Mon Sep 17 00:00:00 2001
From: Lectem <lectem@gmail.com>
Date: Sun, 5 Feb 2017 00:29:07 +0100
Subject: [PATCH 5/5] fix wwylele's comment and use typename in templates

---
 src/core/hle/ipc_helpers.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/core/hle/ipc_helpers.h b/src/core/hle/ipc_helpers.h
index 68508caba..323158bb5 100644
--- a/src/core/hle/ipc_helpers.h
+++ b/src/core/hle/ipc_helpers.h
@@ -53,8 +53,8 @@ public:
     void Push(u32 value) {
         cmdbuf[index++] = value;
     }
-    template <typename First, class... Other>
-    void Push(First first_value, const Other&... other_values) {
+    template <typename First, typename... Other>
+    void Push(const First& first_value, const Other&... other_values) {
         Push(first_value);
         Push(other_values...);
     }
@@ -144,7 +144,7 @@ public:
     template <typename T>
     void Pop(T& value);
 
-    template <typename First, class... Other>
+    template <typename First, typename... Other>
     void Pop(First& first_value, Other&... other_values);
 
     Kernel::Handle PopHandle();
@@ -211,7 +211,7 @@ void RequestParser::Pop(T& value) {
     value = Pop<T>();
 }
 
-template <typename First, class... Other>
+template <typename First, typename... Other>
 void RequestParser::Pop(First& first_value, Other&... other_values) {
     first_value = Pop<First>();
     Pop(other_values...);