Re: [hatari-devel] Control socket server

[ Thread Index | Date Index | More lists.tuxfamily.org/hatari-devel Archives ]


Hi,

FIFO command support was easy enough that I already implemented it.

See the attached patch.

Could you test building the patch on OSX & Windows and see whether
it works correctly (supports this on OSX, doesn't on Windows)?

Usage:
$ rm cmd-fifo
$ hatari --confirm-quit off --cmd-fifo cmd-fifo &

$ echo "help" > cmd-fifo
$ echo "hatari-shortcut help" > cmd-fifo
$ echo "hatari-shortcut coldreset" > cmd-fifo
$ echo "hatari-shortcut quit" > cmd-fifo


	- Eero

On 05/01/2018 04:45 PM, Eero Tamminen wrote:
On 05/01/2018 02:29 PM, tin@xxxxxxxxx wrote:
I have a question about the reasoning behind Hataris control sockets' need for an external dedicated server.

Is there any reason to not serve/create the socket in Hatari itself (other than additional complexity)? Is it planned to control more than one instance from one server?

If user accidentally quits Hatari instance which is running
inside Python GUI, GUI just starts a new instance of Hatari.

I.e. the original use-case for control socket was 1:N-hataris,
whereas you're more interested about N:1-hatari.


I'm currently playing with this and it seems a bit of a hassle to have every control application implementation set up a server (like e.g. the PythonUI does).

You don't need to re-implement it, just use hconsole.

You start your own code from the "example.py" in tools/hconsole/.


My current testbed is a dead simple edit&continue
workflow involving Sublime and an Hatari debug script (aka "magic").
To make things easier on the Editors side, I patched Hatari for OSX (domain socket) & Win (IP socket) to serve the connection. That way even a simple shell script can control Hatari nicely along the lines of echo+nc.

It would be nice to simplify the control sockets for users/developers - especially the "new" breed of text editors (Atom, Sublime, VSCode) lend themselves nicely to send commands to Hatari. Of course the extensions could spawn their own server, but it seems a bit of an overkill and adds another hurdle.

If you need just write-only way to give commands to Hatari,
you don't need a socket, a non-blocking FIFO file would be
enough for that (see "man fifo").

That should work both on OSX & Linux, but I don't know whether
Windows supports something like that?

If you try that out and provide tested patch taking the FIFO
path e.g. from environment variable, I can add rest of the
stuff needed (command line option, doc updates).
diff -r 20ad5f464dc2 src/includes/control.h
--- a/src/includes/control.h	Tue May 01 16:36:56 2018 +0300
+++ b/src/includes/control.h	Tue May 01 17:22:21 2018 +0300
@@ -14,6 +14,7 @@
 /* supported only on BSD compatible / POSIX compliant systems */
 #if HAVE_UNIX_DOMAIN_SOCKETS
 extern bool Control_CheckUpdates(void);
+extern const char* Control_SetFifo(const char *fifopath);
 extern const char* Control_SetSocket(const char *socketpath);
 extern void Control_ReparentWindow(int width, int height, bool noembed);
 #else
diff -r 20ad5f464dc2 src/options.c
--- a/src/options.c	Tue May 01 16:36:56 2018 +0300
+++ b/src/options.c	Tue May 01 17:22:21 2018 +0300
@@ -177,6 +177,7 @@
 	OPT_SAVECONFIG,
 	OPT_PARACHUTE,
 	OPT_CONTROLSOCKET,
+	OPT_CMDFIFO,
 	OPT_LOGFILE,
 	OPT_LOGLEVEL,
 	OPT_ALERTLEVEL,
@@ -462,7 +463,9 @@
 	  NULL, "Disable SDL parachute to get Hatari core dumps" },
 #if HAVE_UNIX_DOMAIN_SOCKETS
 	{ OPT_CONTROLSOCKET, NULL, "--control-socket",
-	  "<file>", "Hatari reads options from given socket at run-time" },
+	  "<file>", "Hatari connects to given socket for control" },
+	{ OPT_CMDFIFO, NULL, "--cmd-fifo",
+	  "<file>", "Hatari reads commands from given fifo" },
 #endif
 	{ OPT_LOGFILE, NULL, "--log-file",
 	  "<file>", "Save log output to <file> (default=stderr)" },
@@ -2058,6 +2061,15 @@
 			}
 			break;
 
+		case OPT_CMDFIFO:
+			i += 1;
+			errstr = Control_SetFifo(argv[i]);
+			if (errstr)
+			{
+				return Opt_ShowError(OPT_CMDFIFO, argv[i], errstr);
+			}
+			break;
+
 		case OPT_LOGFILE:
 			i += 1;
 			ok = Opt_StrCpy(OPT_LOGFILE, false, ConfigureParams.Log.sLogFileName,
diff -r 20ad5f464dc2 src/control.c
--- a/src/control.c	Tue May 01 16:36:56 2018 +0300
+++ b/src/control.c	Tue May 01 17:22:21 2018 +0300
@@ -12,7 +12,9 @@
 
 #if HAVE_UNIX_DOMAIN_SOCKETS
 # include <sys/socket.h>
+# include <sys/stat.h>	/* mkfifo() */
 # include <sys/un.h>
+# include <fcntl.h>
 #endif
 
 #include <sys/types.h>
@@ -366,7 +368,12 @@
 
 #if HAVE_UNIX_DOMAIN_SOCKETS
 
-/* socket from which control command line options are read */
+/* one-way fifo from which to read commands / options */
+static int ControlFifo;
+
+/* two-way socket from which control commands are read,
+ * and where command responses (if any) are written to
+ */
 static int ControlSocket;
 
 /* pre-declared local functions */
@@ -389,6 +396,22 @@
 	ssize_t bytes;
 	int status, sock;
 
+	if (ControlFifo) {
+		/* assume whole command can be read in one go */
+		bytes = read(ControlFifo, buffer, sizeof(buffer)-1);
+		if (bytes < 0) {
+			perror("command FIFO read");
+			return false;
+		}
+		if (bytes == 0) {
+			/* non-blocking read, nothing to read */
+			return false;
+		}
+		buffer[bytes] = '\0';
+		Control_ProcessBuffer(buffer);
+		return false;
+	}
+
 	/* socket of file? */
 	if (ControlSocket) {
 		sock = ControlSocket;
@@ -431,13 +454,13 @@
 		
 		/* assume whole command can be read in one go */
 		bytes = read(sock, buffer, sizeof(buffer)-1);
-		if (bytes < 0)
-		{
+		if (bytes < 0) {
 			perror("Control socket read");
 			return false;
 		}
 		if (bytes == 0) {
 			/* closed */
+			fprintf(stderr, "ready control socket with 0 bytes available -> close socket\n");
 			close(ControlSocket);
 			ControlSocket = 0;
 			return false;
@@ -453,6 +476,39 @@
 
 /*-----------------------------------------------------------------------*/
 /**
+ * Open given command FIFO
+ * Return NULL for success, otherwise an error string
+ */
+const char *Control_SetFifo(const char *path)
+{
+	int fifo;
+
+	if (ControlSocket) {
+		return "Can't use FIFO at same time with control socket";
+	}
+
+	if (mkfifo(path, S_IRUSR | S_IWUSR)) {
+		perror("FIFO creation");
+		return "Can't create FIFO file";
+	}
+
+	fifo = open(path, O_RDONLY | O_NONBLOCK);
+	if (fifo < 0) {
+		perror("FIFO open");
+		return "opening non-blocking read-only FIFO failed";
+	}
+				
+	if (ControlFifo) {
+		close(ControlFifo);
+	}
+	ControlFifo = fifo;
+	Log_Printf(LOG_INFO, "new command FIFO is '%s'\n", path);
+	return NULL;
+}
+
+
+/*-----------------------------------------------------------------------*/
+/**
  * Open given control socket.
  * Return NULL for success, otherwise an error string
  */
@@ -460,10 +516,13 @@
 {
 	struct sockaddr_un address;
 	int newsock;
+
+	if (ControlFifo) {
+		return "Can't use FIFO at same time with control socket";
+	}
 	
 	newsock = socket(AF_UNIX, SOCK_STREAM, 0);
-	if (newsock < 0)
-	{
+	if (newsock < 0) {
 		perror("socket creation");
 		return "Can't create AF_UNIX socket";
 	}
@@ -472,8 +531,7 @@
 	strncpy(address.sun_path, socketpath, sizeof(address.sun_path));
 	address.sun_path[sizeof(address.sun_path)-1] = '\0';
 	Log_Printf(LOG_INFO, "Connecting to control socket '%s'...\n", address.sun_path);
-	if (connect(newsock, (struct sockaddr *)&address, sizeof(address)) < 0)
-	{
+	if (connect(newsock, (struct sockaddr *)&address, sizeof(address)) < 0) {
 		perror("socket connect");
 		close(newsock);
 		return "connection to control socket failed";


Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/