userspace: Use SOCK_STREAM instead of SOCK_SEQPACKET for WindowServer

This makes more sense if we have longer packages

userspace/programs/Terminal/Terminal.cpp

@@ -126,7 +126,7 @@ void Terminal::run()
 	MUST(m_cursor_buffer.resize(m_font.width() * m_font.height(), m_bg_color));
 	show_cursor();
 
-	m_window->set_key_event_callback([&](LibGUI::EventPacket::KeyEvent event) { on_key_event(event); });
+	m_window->set_key_event_callback([&](LibGUI::EventPacket::KeyEvent::event_t event) { on_key_event(event); });
 
 	const int max_fd = BAN::Math::max(m_shell_info.pts_master, m_window->server_fd());
 	while (!s_shell_exited)
@@ -576,7 +576,7 @@ Rectangle Terminal::putchar(uint8_t ch)
 	return should_invalidate;
 }
 
-void Terminal::on_key_event(LibGUI::EventPacket::KeyEvent event)
+void Terminal::on_key_event(LibGUI::EventPacket::KeyEvent::event_t event)
 {
 	if (event.released())
 		return;

userspace/programs/Terminal/Terminal.h

@@ -43,7 +43,7 @@ private:
 	void hide_cursor();
 	void show_cursor();
 
-	void on_key_event(LibGUI::EventPacket::KeyEvent);
+	void on_key_event(LibGUI::EventPacket::KeyEvent::event_t);
 
 	void start_shell();
 

userspace/programs/WindowServer/Window.cpp

@@ -27,9 +27,8 @@ Window::~Window()
 	munmap(m_fb_addr, client_width() * client_height() * 4);
 	smo_delete(m_smo_key);
 
-	LibGUI::EventPacket event;
-	event.type = LibGUI::EventPacket::Type::DestroyWindow;
-	send(m_client_fd, &event, sizeof(event), 0);
+	LibGUI::EventPacket::DestroyWindowEvent packet;
+	(void)packet.send_serialized(m_client_fd);
 	close(m_client_fd);
 }
 

userspace/programs/WindowServer/WindowServer.cpp

@@ -2,6 +2,7 @@
 #include "WindowServer.h"
 
 #include <BAN/Debug.h>
+#include <BAN/ScopeGuard.h>
 
 #include <LibGUI/Window.h>
 #include <LibInput/KeyboardLayout.h>
@@ -26,107 +27,109 @@ WindowServer::WindowServer(Framebuffer& framebuffer, int32_t corner_radius)
 BAN::ErrorOr<void> WindowServer::set_background_image(BAN::UniqPtr<LibImage::Image> image)
 {
 	if (image->width() != (uint64_t)m_framebuffer.width || image->height() != (uint64_t)m_framebuffer.height)
-		image = TRY(image->resize(m_framebuffer.width, m_framebuffer.height));
+		image = TRY(image->resize(m_framebuffer.width, m_framebuffer.height, LibImage::Image::ResizeAlgorithm::Linear));
 	m_background_image = BAN::move(image);
 	invalidate(m_framebuffer.area());
 	return {};
 }
 
-void WindowServer::on_window_packet(int fd, LibGUI::WindowPacket packet)
+void WindowServer::on_window_create(int fd, const LibGUI::WindowPacket::WindowCreate& packet)
 {
-	switch (packet.type)
+	for (auto& window : m_client_windows)
 	{
-		case LibGUI::WindowPacketType::CreateWindow:
-		{
-			// FIXME: This should be probably allowed
-			for (auto& window : m_client_windows)
-			{
-				if (window->client_fd() == fd)
-				{
-					dwarnln("client {} tried to create window while already owning a window", fd);
-					return;
-				}
-			}
-
-			const size_t window_fb_bytes = packet.create.width * packet.create.height * 4;
-
-			long smo_key = smo_create(window_fb_bytes, PROT_READ | PROT_WRITE);
-			if (smo_key == -1)
-			{
-				dwarnln("smo_create: {}", strerror(errno));
-				break;
-			}
-
-			Rectangle window_area {
-				static_cast<int32_t>((m_framebuffer.width - packet.create.width) / 2),
-				static_cast<int32_t>((m_framebuffer.height - packet.create.height) / 2),
-				static_cast<int32_t>(packet.create.width),
-				static_cast<int32_t>(packet.create.height)
-			};
-
-			packet.create.title[sizeof(packet.create.title) - 1] = '\0';
-
-			// Window::Window(int fd, Rectangle area, long smo_key, BAN::StringView title, const LibFont::Font& font)
-			auto window = MUST(BAN::RefPtr<Window>::create(
-				fd,
-				window_area,
-				smo_key,
-				packet.create.title,
-				m_font
-			));
-			MUST(m_client_windows.push_back(window));
-			set_focused_window(window);
-
-			LibGUI::WindowCreateResponse response;
-			response.framebuffer_smo_key = smo_key;
-			if (send(window->client_fd(), &response, sizeof(response), 0) != sizeof(response))
-			{
-				dwarnln("send: {}", strerror(errno));
-				break;
-			}
-
-			break;
-		}
-		case LibGUI::WindowPacketType::Invalidate:
-		{
-			if (packet.invalidate.width == 0 || packet.invalidate.height == 0)
-				break;
-
-			BAN::RefPtr<Window> target_window;
-			for (auto& window : m_client_windows)
-			{
-				if (window->client_fd() == fd)
-				{
-					target_window = window;
-					break;
-				}
-			}
-			if (!target_window)
-			{
-				dwarnln("client {} tried to invalidate window while not owning a window", fd);
-				break;
-			}
-
-			const int32_t br_x = packet.invalidate.x + packet.invalidate.width - 1;
-			const int32_t br_y = packet.invalidate.y + packet.invalidate.height - 1;
-			if (!target_window->client_size().contains({ br_x, br_y }))
-			{
-				dwarnln("Invalid Invalidate packet parameters");
-				break;
-			}
-
-			invalidate({
-				target_window->client_x() + static_cast<int32_t>(packet.invalidate.x),
-				target_window->client_y() + static_cast<int32_t>(packet.invalidate.y),
-				static_cast<int32_t>(packet.invalidate.width),
-				static_cast<int32_t>(packet.invalidate.height),
-			});
-
-			break;
-		}
-		default:
-			ASSERT_NOT_REACHED();
+		if (window->client_fd() != fd)
+			continue;
+		dwarnln("client with window tried to create another one");
+		return;
 	}
+
+	const size_t window_fb_bytes = packet.width * packet.height * 4;
+
+	long smo_key = smo_create(window_fb_bytes, PROT_READ | PROT_WRITE);
+	if (smo_key == -1)
+	{
+		dwarnln("smo_create: {}", strerror(errno));
+		return;
+	}
+	BAN::ScopeGuard smo_deleter([smo_key] { smo_delete(smo_key); });
+
+	Rectangle window_area {
+		static_cast<int32_t>((m_framebuffer.width - packet.width) / 2),
+		static_cast<int32_t>((m_framebuffer.height - packet.height) / 2),
+		static_cast<int32_t>(packet.width),
+		static_cast<int32_t>(packet.height)
+	};
+
+	// Window::Window(int fd, Rectangle area, long smo_key, BAN::StringView title, const LibFont::Font& font)
+	auto window_or_error = (BAN::RefPtr<Window>::create(
+		fd,
+		window_area,
+		smo_key,
+		packet.title,
+		m_font
+	));
+	if (window_or_error.is_error())
+	{
+		dwarnln("could not create window for client: {}", window_or_error.error());
+		return;
+	}
+	auto window = window_or_error.release_value();
+
+	if (auto ret = m_client_windows.push_back(window); ret.is_error())
+	{
+		dwarnln("could not create window for client: {}", ret.error());
+		return;
+	}
+	BAN::ScopeGuard window_popper([&] { m_client_windows.pop_back(); });
+
+	LibGUI::WindowPacket::WindowCreateResponse response;
+	response.smo_key = smo_key;
+	if (auto ret = response.send_serialized(fd); ret.is_error())
+	{
+		dwarnln("could not respond to window create request: {}", ret.error());
+		return;
+	}
+
+	smo_deleter.disable();
+	window_popper.disable();
+
+	set_focused_window(window);
+}
+
+void WindowServer::on_window_invalidate(int fd, const LibGUI::WindowPacket::WindowInvalidate& packet)
+{
+	if (packet.width == 0 || packet.height == 0)
+		return;
+
+	BAN::RefPtr<Window> target_window;
+	for (auto& window : m_client_windows)
+	{
+		if (window->client_fd() != fd)
+			continue;
+		target_window = window;
+		break;
+	}
+
+	if (!target_window)
+	{
+		dwarnln("client tried to invalidate window while not owning a window");
+		return;
+	}
+
+	const int32_t br_x = packet.x + packet.width - 1;
+	const int32_t br_y = packet.y + packet.height - 1;
+	if (!target_window->client_size().contains({ br_x, br_y }))
+	{
+		dwarnln("invalid Invalidate packet parameters");
+		return;
+	}
+
+	invalidate({
+		target_window->client_x() + static_cast<int32_t>(packet.x),
+		target_window->client_y() + static_cast<int32_t>(packet.y),
+		static_cast<int32_t>(packet.width),
+		static_cast<int32_t>(packet.height),
+	});
 }
 
 void WindowServer::on_key_event(LibInput::KeyEvent event)
@@ -173,10 +176,10 @@ void WindowServer::on_key_event(LibInput::KeyEvent event)
 
 	if (m_focused_window)
 	{
-		LibGUI::EventPacket packet;
-		packet.type = LibGUI::EventPacket::Type::KeyEvent;
-		packet.key_event = event;
-		send(m_focused_window->client_fd(), &packet, sizeof(packet), 0);
+		LibGUI::EventPacket::KeyEvent packet;
+		packet.event = event;
+		if (auto ret = packet.send_serialized(m_focused_window->client_fd()); ret.is_error())
+			dwarnln("could not send key event: {}", ret.error());
 	}
 }
 
@@ -207,20 +210,26 @@ void WindowServer::on_mouse_button(LibInput::MouseButtonEvent event)
 	else if (!event.pressed && event.button == LibInput::MouseButton::Left && target_window->close_button_area().contains(m_cursor))
 	{
 		// NOTE: we always have target window if code reaches here
-		LibGUI::EventPacket packet;
-		packet.type = LibGUI::EventPacket::Type::CloseWindow;
-		send(m_focused_window->client_fd(), &packet, sizeof(packet), 0);
+		LibGUI::EventPacket::CloseWindowEvent packet;
+		if (auto ret = packet.send_serialized(m_focused_window->client_fd()); ret.is_error())
+		{
+			dwarnln("could not send close window event: {}", ret.error());
+			return;
+		}
 	}
 	else if (target_window->client_area().contains(m_cursor))
 	{
 		// NOTE: we always have target window if code reaches here
-		LibGUI::EventPacket packet;
-		packet.type = LibGUI::EventPacket::Type::MouseButtonEvent;
-		packet.mouse_button_event.button = event.button;
-		packet.mouse_button_event.pressed = event.pressed;
-		packet.mouse_button_event.x = m_cursor.x - m_focused_window->client_x();
-		packet.mouse_button_event.y = m_cursor.y - m_focused_window->client_y();
-		send(m_focused_window->client_fd(), &packet, sizeof(packet), 0);
+		LibGUI::EventPacket::MouseButtonEvent packet;
+		packet.event.button = event.button;
+		packet.event.pressed = event.pressed;
+		packet.event.x = m_cursor.x - m_focused_window->client_x();
+		packet.event.y = m_cursor.y - m_focused_window->client_y();
+		if (auto ret = packet.send_serialized(m_focused_window->client_fd()); ret.is_error())
+		{
+			dwarnln("could not send mouse button event event: {}", ret.error());
+			return;
+		}
 	}
 }
 
@@ -265,11 +274,14 @@ void WindowServer::on_mouse_move(LibInput::MouseMoveEvent event)
 
 	if (m_focused_window)
 	{
-		LibGUI::EventPacket packet;
-		packet.type = LibGUI::EventPacket::Type::MouseMoveEvent;
-		packet.mouse_move_event.x = m_cursor.x - m_focused_window->client_x();
-		packet.mouse_move_event.y = m_cursor.y - m_focused_window->client_y();
-		send(m_focused_window->client_fd(), &packet, sizeof(packet), 0);
+		LibGUI::EventPacket::MouseMoveEvent packet;
+		packet.event.x = m_cursor.x - m_focused_window->client_x();
+		packet.event.y = m_cursor.y - m_focused_window->client_y();
+		if (auto ret = packet.send_serialized(m_focused_window->client_fd()); ret.is_error())
+		{
+			dwarnln("could not send mouse move event event: {}", ret.error());
+			return;
+		}
 	}
 }
 
@@ -277,10 +289,13 @@ void WindowServer::on_mouse_scroll(LibInput::MouseScrollEvent event)
 {
 	if (m_focused_window)
 	{
-		LibGUI::EventPacket packet;
-		packet.type = LibGUI::EventPacket::Type::MouseScrollEvent;
-		packet.mouse_scroll_event = event;
-		send(m_focused_window->client_fd(), &packet, sizeof(packet), 0);
+		LibGUI::EventPacket::MouseScrollEvent packet;
+		packet.event.scroll = event.scroll;
+		if (auto ret = packet.send_serialized(m_focused_window->client_fd()); ret.is_error())
+		{
+			dwarnln("could not send mouse scroll event event: {}", ret.error());
+			return;
+		}
 	}
 }
 
@@ -595,19 +610,19 @@ Rectangle WindowServer::cursor_area() const
 
 void WindowServer::add_client_fd(int fd)
 {
-	MUST(m_client_fds.push_back(fd));
+	if (auto ret = m_client_data.emplace(fd); ret.is_error())
+	{
+		dwarnln("could not add client: {}", ret.error());
+		return;
+	}
 }
 
 void WindowServer::remove_client_fd(int fd)
 {
-	for (size_t i = 0; i < m_client_fds.size(); i++)
-	{
-		if (m_client_fds[i] == fd)
-		{
-			m_client_fds.remove(i);
-			break;
-		}
-	}
+	auto it = m_client_data.find(fd);
+	if (it == m_client_data.end())
+		return;
+	m_client_data.remove(it);
 
 	for (size_t i = 0; i < m_client_windows.size(); i++)
 	{
@@ -635,7 +650,7 @@ void WindowServer::remove_client_fd(int fd)
 int WindowServer::get_client_fds(fd_set& fds) const
 {
 	int max_fd = 0;
-	for (int fd : m_client_fds)
+	for (const auto& [fd, _] : m_client_data)
 	{
 		FD_SET(fd, &fds);
 		max_fd = BAN::Math::max(max_fd, fd);
@@ -643,13 +658,13 @@ int WindowServer::get_client_fds(fd_set& fds) const
 	return max_fd;
 }
 
-void WindowServer::for_each_client_fd(const BAN::Function<BAN::Iteration(int)>& callback)
+void WindowServer::for_each_client_fd(const BAN::Function<BAN::Iteration(int, ClientData&)>& callback)
 {
 	m_deleted_window = false;
-	for (int fd : m_client_fds)
+	for (auto& [fd, cliend_data] : m_client_data)
 	{
 		if (m_deleted_window)
 			break;
-		callback(fd);
+		callback(fd, cliend_data);
 	}
 }

userspace/programs/WindowServer/WindowServer.h

@@ -18,12 +18,20 @@
 
 class WindowServer
 {
+public:
+	struct ClientData
+	{
+		size_t packet_buffer_nread = 0;
+		BAN::Vector<uint8_t> packet_buffer;
+	};
+
 public:
 	WindowServer(Framebuffer& framebuffer, int32_t corner_radius);
 
 	BAN::ErrorOr<void> set_background_image(BAN::UniqPtr<LibImage::Image>);
 
-	void on_window_packet(int fd, LibGUI::WindowPacket);
+	void on_window_create(int fd, const LibGUI::WindowPacket::WindowCreate&);
+	void on_window_invalidate(int fd, const LibGUI::WindowPacket::WindowInvalidate&);
 
 	void on_key_event(LibInput::KeyEvent event);
 	void on_mouse_button(LibInput::MouseButtonEvent event);
@@ -39,14 +47,15 @@ public:
 	void add_client_fd(int fd);
 	void remove_client_fd(int fd);
 	int get_client_fds(fd_set& fds) const;
-	void for_each_client_fd(const BAN::Function<BAN::Iteration(int)>& callback);
+	void for_each_client_fd(const BAN::Function<BAN::Iteration(int, ClientData&)>& callback);
 
 	bool is_stopped() const { return m_is_stopped; }
 
 private:
 	Framebuffer& m_framebuffer;
 	BAN::Vector<BAN::RefPtr<Window>> m_client_windows;
-	BAN::Vector<int> m_client_fds;
+
+	BAN::HashMap<int, ClientData> m_client_data;
 
 	const int32_t m_corner_radius;
 

userspace/programs/WindowServer/main.cpp

@@ -114,7 +114,7 @@ int open_server_fd()
 	if (stat(LibGUI::s_window_server_socket.data(), &st) != -1)
 		unlink(LibGUI::s_window_server_socket.data());
 
-	int server_fd = socket(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0);
+	int server_fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
 	if (server_fd == -1)
 	{
 		perror("socket");
@@ -178,12 +178,6 @@ int main()
 
 	dprintln("Window server started");
 
-	size_t window_packet_sizes[LibGUI::WindowPacketType::COUNT] {};
-	window_packet_sizes[LibGUI::WindowPacketType::INVALID]		= 0;
-	window_packet_sizes[LibGUI::WindowPacketType::CreateWindow]	= sizeof(LibGUI::WindowCreatePacket);
-	window_packet_sizes[LibGUI::WindowPacketType::Invalidate]	= sizeof(LibGUI::WindowInvalidatePacket);
-	static_assert(LibGUI::WindowPacketType::COUNT == 3);
-
 	auto config = parse_config();
 
 	WindowServer window_server(framebuffer, config.corner_radius);
@@ -281,13 +275,49 @@ int main()
 		}
 
 		window_server.for_each_client_fd(
-			[&](int fd) -> BAN::Iteration
+			[&](int fd, WindowServer::ClientData& client_data) -> BAN::Iteration
 			{
 				if (!FD_ISSET(fd, &fds))
 					return BAN::Iteration::Continue;
 
-				LibGUI::WindowPacket packet;
-				ssize_t nrecv = recv(fd, &packet, sizeof(packet), 0);
+				if (client_data.packet_buffer.empty())
+				{
+					uint32_t packet_size;
+					const ssize_t nrecv = recv(fd, &packet_size, sizeof(uint32_t), 0);
+					if (nrecv < 0)
+						dwarnln("recv: {}", strerror(errno));
+					if (nrecv > 0 && nrecv != sizeof(uint32_t))
+						dwarnln("could not read packet size with a single recv call, closing connection...");
+					if (nrecv != sizeof(uint32_t))
+					{
+						window_server.remove_client_fd(fd);
+						return BAN::Iteration::Continue;
+					}
+
+					if (packet_size < 4)
+					{
+						dwarnln("client sent invalid packet, closing connection...");
+						return BAN::Iteration::Continue;
+					}
+
+					// this is a bit harsh, but i don't want to work on skipping streaming packets
+					if (client_data.packet_buffer.resize(packet_size).is_error())
+					{
+						dwarnln("could not allocate memory for client packet, closing connection...");
+						window_server.remove_client_fd(fd);
+						return BAN::Iteration::Continue;
+					}
+
+					client_data.packet_buffer_nread = 0;
+					return BAN::Iteration::Continue;
+				}
+
+				const ssize_t nrecv = recv(
+					fd,
+					client_data.packet_buffer.data() + client_data.packet_buffer_nread,
+					client_data.packet_buffer.size() - client_data.packet_buffer_nread,
+					0
+				);
 				if (nrecv < 0)
 					dwarnln("recv: {}", strerror(errno));
 				if (nrecv <= 0)
@@ -296,12 +326,30 @@ int main()
 					return BAN::Iteration::Continue;
 				}
 
-				if (packet.type == LibGUI::WindowPacketType::INVALID || packet.type >= LibGUI::WindowPacketType::COUNT)
-					dwarnln("Invalid WindowPacket (type {})", (int)packet.type);
-				if (static_cast<size_t>(nrecv) != window_packet_sizes[packet.type])
-					dwarnln("Invalid WindowPacket size (type {}, size {})", (int)packet.type, nrecv);
-				else
-					window_server.on_window_packet(fd, packet);
+				client_data.packet_buffer_nread += nrecv;
+				if (client_data.packet_buffer_nread < client_data.packet_buffer.size())
+					return BAN::Iteration::Continue;
+
+				ASSERT(client_data.packet_buffer.size() >= sizeof(uint32_t));
+
+				switch (*reinterpret_cast<LibGUI::PacketType*>(client_data.packet_buffer.data()))
+				{
+					case LibGUI::PacketType::WindowCreate:
+					{
+						if (auto ret = LibGUI::WindowPacket::WindowCreate::deserialize(client_data.packet_buffer.span()); !ret.is_error())
+							window_server.on_window_create(fd, ret.release_value());
+						break;
+					}
+					case LibGUI::PacketType::WindowInvalidate:
+						if (auto ret = LibGUI::WindowPacket::WindowInvalidate::deserialize(client_data.packet_buffer.span()); !ret.is_error())
+							window_server.on_window_invalidate(fd, ret.release_value());
+						break;
+					default:
+						dprintln("unhandled packet type: {}", *reinterpret_cast<uint32_t*>(client_data.packet_buffer.data()));
+				}
+
+				client_data.packet_buffer.clear();
+				client_data.packet_buffer_nread = 0;
 				return BAN::Iteration::Continue;
 			}
 		);