fixup! Add display server.

This reverts commit 0fef08bbc4.

chvideomode/chvideomode.1

@@ -1,4 +1,4 @@
-.Dd February 6, 2017
+.Dd June 16, 2023
 .Dt CHVIDEOMODE 1
 .Os
 .Sh NAME
@@ -63,7 +63,9 @@ The interactive key commands are as follows:
 Move the selection up 1 item.
 .It Down Arrow
 Move the selection down 1 item.
-.It Escape
+.It 0 - 9
+Select item of the specified number.
+.It q , Q
 Quit without changing video mode.
 .It Enter
 Change the video mode to the current selection.

chvideomode/chvideomode.c

@@ -20,8 +20,6 @@
 
 #include <sys/display.h>
 #include <sys/ioctl.h>
-#include <sys/keycodes.h>
-#include <sys/termmode.h>
 #include <sys/wait.h>
 
 #include <errno.h>
@@ -29,6 +27,7 @@
 #include <fcntl.h>
 #include <getopt.h>
 #include <inttypes.h>
+#include <signal.h>
 #include <stdarg.h>
 #include <stdbool.h>
 #include <stdint.h>
@@ -38,6 +37,17 @@
 #include <termios.h>
 #include <unistd.h>
 
+struct termios saved;
+
+static void restore_terminal(int sig)
+{
+	if ( tcsetattr(0, TCSANOW, &saved) )
+		err(1, "tcsetattr");
+	// Re-raise the signal. As we set SA_RESETHAND this will not run the handler
+	// again but rather fall back to default.
+	raise(sig);
+}
+
 static bool set_current_mode(const struct tiocgdisplay* display,
                              struct dispmsg_crtc_mode mode)
 {
@@ -202,18 +212,15 @@ static bool select_mode(struct dispmsg_crtc_mode* modes,
                         int mode_set_error,
                         struct dispmsg_crtc_mode* mode)
 {
+	if ( !isatty(0) )
+		errx(1, "Interactive menu requires stdin to be a terminal");
+
 	int num_modes_display_length = 1;
 	for ( size_t i = num_modes; 10 <= i; i /= 10 )
 		num_modes_display_length++;
 
-	size_t selection;
-	bool decided;
-	bool first_render;
-	struct wincurpos render_at;
-	selection = 0;
-	decided = false;
-	first_render = true;
-	memset(&render_at, 0, sizeof(render_at));
+	size_t selection = 0;
+	bool decided = false;
 	while ( !decided )
 	{
 		fflush(stdout);
@@ -225,13 +232,6 @@ static bool select_mode(struct dispmsg_crtc_mode* modes,
 			ws.ws_row = 25;
 		}
 
-		struct wincurpos wcp;
-		if ( tcgetwincurpos(1, &wcp) != 0 )
-		{
-			wcp.wcp_col = 1;
-			wcp.wcp_row = 1;
-		}
-
 		size_t off = 1; // The "Please select ..." line at the top.
 		if ( mode_set_error )
 			off++;
@@ -245,36 +245,18 @@ static bool select_mode(struct dispmsg_crtc_mode* modes,
 			how_many = how_many_available;
 		size_t lines_on_screen = off + how_many;
 
-		if ( first_render )
-		{
-			while ( wcp.wcp_row &&
-			        ws.ws_row - (wcp.wcp_row + 1) < lines_on_screen )
-			{
-				printf("\e[S");
-				printf("\e[%juH", 1 + (uintmax_t) wcp.wcp_row);
-				wcp.wcp_row--;
-				wcp.wcp_col = 1;
-			}
-			render_at = wcp;
-			first_render = false;
-		}
-
 		printf("\e[m");
-		printf("\e[%juH", 1 + (uintmax_t) render_at.wcp_row);
 		printf("\e[2K");
 
 		if ( mode_set_error )
 			printf("Error: Could not set desired mode: %s\n",
 			       strerror(mode_set_error));
-		printf("Please select one of these video modes or press ESC to "
-		       "abort.\n");
+		printf("Please select one of these video modes or press Q to abort.\n");
 
 		for ( size_t i = 0; i < how_many; i++ )
 		{
 			size_t index = from + i;
-			size_t screenline = off + index - from;
 			const char* color = index == selection ? "\e[31m" : "\e[m";
-			printf("\e[%zuH", 1 + render_at.wcp_row + screenline);
 			printf("%s", color);
 			printf("\e[2K");
 			printf(" [%-*zu] ", num_modes_display_length, index);
@@ -288,77 +270,108 @@ static bool select_mode(struct dispmsg_crtc_mode* modes,
 			else
 				printf("(unknown video device feature)");
 			printf("\e[m");
+			if ( i + 1 < how_many )
+				printf("\n");
 		}
 
 		printf("\e[J");
 		fflush(stdout);
 
-		unsigned int oldtermmode;
-		if ( gettermmode(0, &oldtermmode) < 0 )
-			err(1, "gettermmode");
+		// Block delivery of SIGTSTP during menu to avoid having to deal with
+		// complex interactions between signals and terminal settings.
+		sigset_t sigtstp;
+		sigemptyset(&sigtstp);
+		sigaddset(&sigtstp, SIGTSTP);
+		sigprocmask(SIG_BLOCK, &sigtstp, NULL);
 
-		if ( settermmode(0, TERMMODE_KBKEY | TERMMODE_UNICODE |
-		                    TERMMODE_SIGNAL) < 0 )
-			err(1, "settermmode");
+		if ( tcgetattr(0, &saved) )
+			err(1, "tcgetattr");
+
+		// Revert back to normal terminal settings before dying on a signal.
+		struct sigaction sa = {0};
+		sa.sa_handler = restore_terminal;
+		sa.sa_flags = SA_RESETHAND; // The handler should only run once.
+		sigaction(SIGINT, &sa, NULL);
+		sigaction(SIGQUIT, &sa, NULL);
+		sigaction(SIGTERM, &sa, NULL);
+
+		struct termios altered = saved;
+		altered.c_lflag &= ~(ECHO | ICANON);
+		if ( tcsetattr(0, TCSANOW, &altered) )
+			err(1, "tcsetattr");
 
 		bool redraw = false;
 		while ( !redraw && !decided )
 		{
-			uint32_t codepoint;
-			ssize_t numbytes = read(0, &codepoint, sizeof(codepoint));
-			if ( numbytes < 0 )
-				err(1, "read");
+			int byte = fgetc(stdin);
 
-			int kbkey = KBKEY_DECODE(codepoint);
-			if ( kbkey )
+			if ( byte == '\e' )
 			{
-				switch ( kbkey )
+				switch ( fgetc(stdin) )
 				{
-				case KBKEY_ESC:
-					if ( settermmode(0, oldtermmode) < 0 )
-						err(1, "settermmode");
+				case 'O': fgetc(stdin); break; // \eO is followed by one byte
+				case '[':
+				{
+					// Sequence can have numbers separated by a semicolon before
+					// the final character, so read until a non-digit
+					// non-semicolon is found.
+					size_t length = 1;
+					while ( (byte = fgetc(stdin)) &&
+					        (('0' <= byte && byte <= '9') || byte == ';' ) )
+						length++;
+
+					if ( length == 1 && byte == 'A' ) // Up key
+					{
+						if ( selection )
+							selection--;
+						else
+							selection = num_modes - 1;
+						redraw = true;
+					}
+					else if ( length == 1 && byte == 'B' ) // Down key
+					{
+						if ( selection + 1 == num_modes )
+							selection = 0;
+						else
+							selection++;
+						redraw = true;
+					}
+					break;
+				}
+				}
+			}
+			else if ( '0' <= byte && byte <= '9' )
+			{
+				uint32_t requested = byte - '0';
+				if ( requested < num_modes )
+				{
+					selection = requested;
+					redraw = true;
+				}
+			}
+			else
+			{
+				switch ( byte )
+				{
+				case 'q':
+				case 'Q':
+					if ( tcsetattr(0, TCSANOW, &saved) )
+						err(1, "tcsetattr");
 					printf("\n");
 					return false;
-					break;
-				case KBKEY_UP:
-					if ( selection )
-						selection--;
-					else
-						selection = num_modes -1;
-					redraw = true;
-					break;
-				case KBKEY_DOWN:
-					if ( selection + 1 == num_modes )
-						selection = 0;
-					else
-						selection++;
-					redraw = true;
-					break;
-				case KBKEY_ENTER:
-					if ( settermmode(0, oldtermmode) < 0 )
-						err(1, "settermmode");
-					fgetc(stdin);
+				case '\n':
 					printf("\n");
 					decided = true;
 					break;
 				}
 			}
-			else
-			{
-				if ( L'0' <= codepoint && codepoint <= '9' )
-				{
-					uint32_t requested = codepoint - '0';
-					if ( requested < num_modes )
-					{
-						selection = requested;
-						redraw = true;
-					}
-				}
-			}
 		}
 
-		if ( settermmode(0, oldtermmode) < 0 )
-			err(1, "settermmode");
+		if ( redraw )
+			printf("\e[%zuF", lines_on_screen - 1);
+
+		if ( tcsetattr(0, TCSANOW, &saved) )
+			err(1, "tcsetattr");
 	}
 
 	*mode = modes[selection];
@@ -498,8 +511,7 @@ int main(int argc, char* argv[])
 	}
 
 	struct tiocgdisplay display;
-	struct tiocgdisplays gdisplays;
-	memset(&gdisplays, 0, sizeof(gdisplays));
+	struct tiocgdisplays gdisplays = {0};
 	gdisplays.count = 1;
 	gdisplays.displays = &display;
 	if ( ioctl(1, TIOCGDISPLAYS, &gdisplays) < 0 || gdisplays.count == 0 )

display/connection.c

@@ -227,8 +227,8 @@ CONNECTION_MESSAGE_HANDLER_SERVER(chkblayout)
 		event.error = errno;
 
 	struct display_packet_header header;
-	header.message_id = EVENT_ACK;
-	header.message_length = sizeof(event);
+	header.id = EVENT_ACK;
+	header.size = sizeof(event);
 
 	connection_schedule_transmit(connection, &header, sizeof(header));
 	connection_schedule_transmit(connection, &event, sizeof(event));
@@ -289,17 +289,17 @@ void connection_can_read(struct connection* connection,
 		connection->packet_header_received += amount;
 	}
 
-	size_t packet_length = connection->packet_header.message_length;
+	size_t packet_size = connection->packet_header.size;
 
 	// TODO: Check allocation and protect against too big buffers.
 	if ( !connection->packet )
-		connection->packet = (unsigned char*) malloc(packet_length);
+		connection->packet = malloc(packet_size);
 
-	while ( connection->packet_received < packet_length )
+	while ( connection->packet_received < packet_size )
 	{
 		ssize_t amount = read(connection->fd,
 		                      connection->packet + connection->packet_received,
-		                      packet_length - connection->packet_received);
+		                      packet_size - connection->packet_received);
 		if ( amount < 0 && (errno == EAGAIN || errno == EWOULDBLOCK) )
 			return;
 		if ( amount < 0 || amount == 0 )
@@ -307,14 +307,14 @@ void connection_can_read(struct connection* connection,
 		connection->packet_received += amount;
 	}
 
-	size_t packet_id = connection->packet_header.message_id;
+	size_t packet_id = connection->packet_header.id;
 
 	if ( packet_id < num_connection_message_handlers )
 	{
 		struct connection_message_handler_registration* handler =
 			&connection_message_handlers[packet_id];
 		unsigned char* auxiliary = connection->packet + handler->message_size;
-		size_t auxiliary_size = packet_length - handler->message_size;
+		size_t auxiliary_size = packet_size - handler->message_size;
 		handler->handler(connection, connection->packet,
 		                 auxiliary, auxiliary_size, server);
 	}

display/display-code.c

@@ -567,8 +567,8 @@ void display_keyboard_event(struct display* display, uint32_t codepoint)
 	event.codepoint = codepoint;
 
 	struct display_packet_header header;
-	header.message_id = EVENT_KEYBOARD;
-	header.message_length = sizeof(event);
+	header.id = EVENT_KEYBOARD;
+	header.size = sizeof(event);
 
 	assert(window->connection);
 

display/window.c

@@ -308,8 +308,8 @@ void window_quit(struct window* window)
 	event.window_id = window->window_id;
 
 	struct display_packet_header header;
-	header.message_id = EVENT_QUIT;
-	header.message_length = sizeof(event);
+	header.id = EVENT_QUIT;
+	header.size = sizeof(event);
 
 	assert(window->connection);
 
@@ -550,8 +550,8 @@ void window_notify_client_resize(struct window* window)
 	event.height = window_client_buffer(window).yres;
 
 	struct display_packet_header header;
-	header.message_id = EVENT_RESIZE;
-	header.message_length = sizeof(event);
+	header.id = EVENT_RESIZE;
+	header.size = sizeof(event);
 
 	assert(window->connection);
 

libdisplay/include/display-protocol.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2015, 2016 Jonas 'Sortie' Termansen.
+ * Copyright (c) 2014, 2015, 2016, 2023 Jonas 'Sortie' Termansen.
  * Copyright (c) 2023 Juhani 'nortti' Krekelä.
  *
  * Permission to use, copy, modify, and distribute this software for any
@@ -25,8 +25,8 @@
 
 struct display_packet_header
 {
-	uint32_t message_id;
-	uint32_t message_length;
+	uint32_t id;
+	uint32_t size;
 };
 
 #define DISPLAY_CREATE_WINDOW 0
@@ -64,7 +64,7 @@ struct display_render_window
 struct display_title_window
 {
 	uint32_t window_id;
-	/* An UTF-8 string follows */
+	/* A non-terminated UTF-8 string follows */
 };
 
 #define DISPLAY_SHOW_WINDOW 5
@@ -122,7 +122,7 @@ struct event_keyboard
 struct event_ack
 {
 	uint32_t id;
-	int error;
+	int32_t error;
 };
 
 #endif

libdisplay/include/display.h

@@ -36,11 +36,8 @@ int display_spawn(int argc, char** argv);
 struct display_connection* display_connect(const char* socket_path);
 struct display_connection* display_connect_default(void);
 void display_disconnect(struct display_connection* connection);
-
 int display_connection_fd(struct display_connection* connection);
-
 void display_shutdown(struct display_connection* connection, uint32_t code);
-
 void display_create_window(struct display_connection* connection,
                            uint32_t window_id);
 void display_destroy_window(struct display_connection* connection,
@@ -63,7 +60,6 @@ void display_show_window(struct display_connection* connection,
                          uint32_t window_id);
 void display_hide_window(struct display_connection* connection,
                          uint32_t window_id);
-
 void display_chkblayout(struct display_connection* connection,
                         uint32_t id,
                         void* data,

libdisplay/libdisplay.c

@@ -38,8 +38,10 @@
 
 int display_spawn(int argc, char** argv)
 {
-	// TODO: Overflow.
-	char** new_argv = malloc((2 + argc + 1) * sizeof(char*));
+	int length = 2 + 1;
+	if ( __builtin_add_overflow(length, argc, &length) )
+		return errno = EOVERFLOW, -1;
+	char** new_argv = reallocarray(NULL, length, sizeof(char*));
 	if ( !new_argv )
 		return -1;
 	new_argv[0] = (char*) "display";
@@ -58,7 +60,7 @@ static int open_local_client_socket(const char* path, int flags)
 {
 	size_t path_length = strlen(path);
 	size_t addr_size = offsetof(struct sockaddr_un, sun_path) + path_length + 1;
-	struct sockaddr_un* sockaddr = (struct sockaddr_un*) malloc(addr_size);
+	struct sockaddr_un* sockaddr = malloc(addr_size);
 	if ( !sockaddr )
 		return -1;
 	sockaddr->sun_family = AF_LOCAL;
@@ -76,18 +78,17 @@ struct display_connection
 {
 	int fd;
 	struct display_packet_header header;
-	size_t header_bytes;
+	size_t header_got;
 	uint8_t* payload;
-	size_t payload_bytes;
+	size_t payload_got;
 };
 
 struct display_connection* display_connect(const char* socket_path)
 {
 	struct display_connection* connection =
-		(struct display_connection*) malloc(sizeof(struct display_connection));
+		calloc(1, sizeof(struct display_connection));
 	if ( !connection )
 		return NULL;
-	memset(connection, 0, sizeof(*connection));
 	if ( (connection->fd = open_local_client_socket(socket_path, 0)) < 0 )
 		return free(connection), (struct display_connection*) NULL;
 	size_t send_buffer_size = 2 * 1024 * 1024;
@@ -116,26 +117,26 @@ int display_connection_fd(struct display_connection* connection)
 }
 
 static void send_message(struct display_connection* connection,
-                         uint32_t message_id,
+                         uint32_t id,
                          const void* message,
                          size_t message_size,
                          const void* auxiliary,
                          size_t auxiliary_size)
 {
 	struct display_packet_header header;
-	header.message_id = message_id;
-	header.message_length = message_size + auxiliary_size;
+	header.id = id;
+	header.size = message_size + auxiliary_size;
 	writeall(connection->fd, &header, sizeof(header));
 	writeall(connection->fd, message, message_size);
 	writeall(connection->fd, auxiliary, auxiliary_size);
 }
 
 static void send_message_no_aux(struct display_connection* connection,
-                                uint32_t message_id,
+                                uint32_t id,
                                 const void* message,
                                 size_t message_size)
 {
-	send_message(connection, message_id, message, message_size, 0, 0);
+	send_message(connection, id, message, message_size, 0, 0);
 }
 
 void display_shutdown(struct display_connection* connection, uint32_t code)
@@ -197,7 +198,8 @@ void display_title_window(struct display_connection* connection,
 {
 	struct display_title_window msg;
 	msg.window_id = window_id;
-	send_message(connection, DISPLAY_TITLE_WINDOW, &msg, sizeof(msg), title, strlen(title));
+	send_message(connection, DISPLAY_TITLE_WINDOW, &msg, sizeof(msg), title,
+	             strlen(title));
 }
 
 void display_show_window(struct display_connection* connection,
@@ -229,41 +231,39 @@ void display_chkblayout(struct display_connection* connection,
 
 static bool display_read_event(struct display_connection* connection)
 {
-	while ( connection->header_bytes < sizeof(connection->header) )
+	while ( connection->header_got < sizeof(connection->header) )
 	{
 		errno = 0;
-		ssize_t amount = read(connection->fd,
-		                      (uint8_t*) &connection->header + connection->header_bytes,
-		                      sizeof(connection->header) - connection->header_bytes);
+		uint8_t* data = (uint8_t*) &connection->header + connection->header_got;
+		size_t left = sizeof(connection->header) - connection->header_got;
+		ssize_t amount = read(connection->fd, data, left);
 		if ( amount < 0 && (errno == EAGAIN || errno == EWOULDBLOCK) )
 			break;
-		if ( amount < 0 )
-			break;
-		if ( amount == 0 )
+		if ( amount <= 0 )
 			return false;
-		connection->header_bytes += amount;
+		connection->header_got += amount;
 	}
-	if ( connection->header_bytes == sizeof(connection->header) &&
+	if ( connection->header_got == sizeof(connection->header) &&
 	     !connection->payload )
 	{
-		connection->payload = (uint8_t*) malloc(connection->header.message_length);
-		connection->payload_bytes = 0;
+		connection->payload = malloc(connection->header.size);
+		if ( !connection->payload )
+			return false;
+		connection->payload_got = 0;
 	}
-	while ( connection->header_bytes == sizeof(connection->header) &&
+	while ( connection->header_got == sizeof(connection->header) &&
 	        connection->payload &&
-	        connection->payload_bytes < connection->header.message_length )
+	        connection->payload_got < connection->header.size )
 	{
 		errno = 0;
-		ssize_t amount = read(connection->fd,
-		                      connection->payload + connection->payload_bytes,
-		                      connection->header.message_length - connection->payload_bytes);
+		uint8_t* data = connection->payload + connection->payload_got;
+		size_t left = connection->header.size - connection->payload_got;
+		ssize_t amount = read(connection->fd, data, left);
 		if ( amount < 0 && (errno == EAGAIN || errno == EWOULDBLOCK) )
 			break;
-		if ( amount < 0 )
-			break;
-		if ( amount == 0 )
+		if ( amount <= 0 )
 			return false;
-		connection->payload_bytes += amount;
+		connection->payload_got += amount;
 	}
 
 	return true;
@@ -272,16 +272,16 @@ static bool display_read_event(struct display_connection* connection)
 static int display_dispatch_event(struct display_connection* connection,
                                   struct display_event_handlers* handlers)
 {
-	if ( connection->header_bytes == sizeof(connection->header) &&
+	if ( connection->header_got == sizeof(connection->header) &&
 	     connection->payload &&
-	     connection->payload_bytes == connection->header.message_length )
+	     connection->payload_got == connection->header.size )
 	{
-		// TODO: == instead of <= due to warning. Maybe all should be changed to
-		//       use == instead.
-		if ( connection->header.message_id == EVENT_DISCONNECT &&
-		     connection->header.message_length ==/*>=*/ sizeof(struct event_disconnect) )
+		void* payload = connection->payload;
+
+		if ( connection->header.id == EVENT_DISCONNECT &&
+		     connection->header.size == sizeof(struct event_disconnect) )
 		{
-			struct event_disconnect* event = (struct event_disconnect*) connection->payload;
+			struct event_disconnect* event = payload;
 			(void) event;
 			if ( handlers->disconnect_handler )
 				handlers->disconnect_handler(handlers->context);
@@ -289,43 +289,47 @@ static int display_dispatch_event(struct display_connection* connection,
 				exit(0);
 		}
 
-		if ( connection->header.message_id == EVENT_QUIT &&
-		     connection->header.message_length >= sizeof(struct event_quit) )
+		if ( connection->header.id == EVENT_QUIT &&
+		     connection->header.size == sizeof(struct event_quit) )
 		{
-			struct event_quit* event = (struct event_quit*) connection->payload;
+			struct event_quit* event = payload;
 			if ( handlers->quit_handler )
 				handlers->quit_handler(handlers->context, event->window_id);
 			else
 				exit(0);
 		}
 
-		if ( connection->header.message_id == EVENT_RESIZE &&
-		     connection->header.message_length >= sizeof(struct event_resize) )
+		if ( connection->header.id == EVENT_RESIZE &&
+		     connection->header.size == sizeof(struct event_resize) )
 		{
-			struct event_resize* event = (struct event_resize*) connection->payload;
+			struct event_resize* event = payload;
 			if ( handlers->resize_handler )
-				handlers->resize_handler(handlers->context, event->window_id, event->width, event->height);
+				handlers->resize_handler(handlers->context, event->window_id,
+				                         event->width, event->height);
 		}
 
-		if ( connection->header.message_id == EVENT_KEYBOARD &&
-		     connection->header.message_length >= sizeof(struct event_keyboard) )
+		if ( connection->header.id == EVENT_KEYBOARD &&
+		     connection->header.size == sizeof(struct event_keyboard) )
 		{
-			struct event_keyboard* event = (struct event_keyboard*) connection->payload;
+			struct event_keyboard* event = payload;
 			if ( handlers->keyboard_handler )
-				handlers->keyboard_handler(handlers->context, event->window_id, event->codepoint);
+				handlers->keyboard_handler(handlers->context, event->window_id,
+				                           event->codepoint);
 		}
 
-		if ( connection->header.message_id == EVENT_ACK &&
-		     connection->header.message_length >= sizeof(struct event_ack) )
+		if ( connection->header.id == EVENT_ACK &&
+		     connection->header.size == sizeof(struct event_ack) )
 		{
-			struct event_ack* event = (struct event_ack*) connection->payload;
+			struct event_ack* event = payload;
 			if ( handlers->ack_handler )
-				handlers->ack_handler(handlers->context, event->id, event->error);
+				handlers->ack_handler(handlers->context, event->id,
+				                      event->error);
 		}
 
-		connection->header_bytes = 0;
-		free(connection->payload), connection->payload = NULL;
-		connection->payload_bytes = 0;
+		connection->header_got = 0;
+		free(connection->payload);
+		connection->payload = NULL;
+		connection->payload_got = 0;
 
 		return 0;
 	}