[1/1] IPC: add dedicated status socket interface

Currently the only way to get status messages is by calling the
ctrl IPC interface, however this has the disadvantage of only
effectively allowing a single client at a time to read status
messages as they are deleted when read using this interface.

In order to allow multiple clients to read status messages
add a dedicated interface for those messages similar to the
progress interface.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 Kconfig                       |   5 +
 core/Makefile                 |   1 +
 core/network_thread.c         |  10 --
 core/notifier.c               |  14 +++
 core/status_thread.c          | 176 ++++++++++++++++++++++++++++++++++
 core/swupdate.c               |   3 +
 core/util.c                   |  10 ++
 include/status.h              |  23 +++++
 include/status_ipc.h          |  53 ++++++++++
 include/util.h                |   1 +
 ipc/Makefile                  |   2 +-
 ipc/status_ipc.c              |  94 ++++++++++++++++++
 mongoose/mongoose_interface.c |  42 +++++---
 13 files changed, 409 insertions(+), 25 deletions(-)
 create mode 100644 core/status_thread.c
 create mode 100644 include/status.h
 create mode 100644 include/status_ipc.h
 create mode 100644 ipc/status_ipc.c

Hi James,

On 15.08.21 05:46, James Hilliard wrote:
> Currently the only way to get status messages is by calling the
> ctrl IPC interface, however this has the disadvantage of only
> effectively allowing a single client at a time to read status
> messages as they are deleted when read using this interface.

This is true.

> 
> In order to allow multiple clients to read status messages
> add a dedicated interface for those messages similar to the
> progress interface.
> 

However, what I see here is that the code is copied and duplicated. I am 
also concerning about creating an additional socket interface, when 
there is already two sockets, one to control SWUpdate (and send a SWU), 
one to get information about the update. The additional status socker is 
mostly a copy frim the progress and it is also unidirectional, that mean 
clients just want to get informed and they do not send data back. More 
or less, the limits of GET_STATUS led me to create the progress 
interface. The GET_STATUS works also in polling mode, while the progress 
interface sends events to the listen. For external interface, a syslog 
notifier sends the same data to the syslog socket.

If this is really required, I do not like to get duplicated code. It 
should be solved reusing in some way the progress interface (and taking 
care of some compatibility issues that could be raised).

Best regards,
Stefano

> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
>   Kconfig                       |   5 +
>   core/Makefile                 |   1 +
>   core/network_thread.c         |  10 --
>   core/notifier.c               |  14 +++
>   core/status_thread.c          | 176 ++++++++++++++++++++++++++++++++++
>   core/swupdate.c               |   3 +
>   core/util.c                   |  10 ++
>   include/status.h              |  23 +++++
>   include/status_ipc.h          |  53 ++++++++++
>   include/util.h                |   1 +
>   ipc/Makefile                  |   2 +-
>   ipc/status_ipc.c              |  94 ++++++++++++++++++
>   mongoose/mongoose_interface.c |  42 +++++---
>   13 files changed, 409 insertions(+), 25 deletions(-)
>   create mode 100644 core/status_thread.c
>   create mode 100644 include/status.h
>   create mode 100644 include/status_ipc.h
>   create mode 100644 ipc/status_ipc.c
> 
> diff --git a/Kconfig b/Kconfig
> index dc86957..7fc7382 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -215,6 +215,11 @@ config SOCKET_PROGRESS_PATH
>   	help
>   	  Path to the socket progress information is sent to.
>   
> +config SOCKET_STATUS_PATH
> +	string "SWUpdate status socket path"
> +	help
> +	  Path to the socket status information is sent to.
> +
>   config SOCKET_NOTIFIER_DIRECTORY
>   	string "SWUpdate notifier socket directory"
>   	depends on HAVE_FREEBSD
> diff --git a/core/Makefile b/core/Makefile
> index fa30e6e..e6aeba4 100644
> --- a/core/Makefile
> +++ b/core/Makefile
> @@ -22,6 +22,7 @@ obj-y += swupdate.o \
>   	 network_thread.o \
>   	 stream_interface.o \
>   	 progress_thread.o \
> +	 status_thread.o \
>   	 parsing_library.o \
>   	 artifacts_versions.o \
>   	 swupdate_dict.o \
> diff --git a/core/network_thread.c b/core/network_thread.c
> index adaf21c..59f1c15 100644
> --- a/core/network_thread.c
> +++ b/core/network_thread.c
> @@ -106,16 +106,6 @@ static bool is_selection_allowed(const char *software_set, char *running_mode,
>   	return allowed;
>   }
>   
> -static void clean_msg(char *msg, char drop)
> -{
> -	char *lfpos;
> -	lfpos = strchr(msg, drop);
> -	while (lfpos) {
> -		*lfpos = ' ';
> -		lfpos = strchr(msg, drop);
> -	}
> -}
> -
>   static void network_notifier(RECOVERY_STATUS status, int error, int level, const char *msg)
>   {
>   	int len = msg ? strlen(msg) : 0;
> diff --git a/core/notifier.c b/core/notifier.c
> index 810769c..7510f93 100644
> --- a/core/notifier.c
> +++ b/core/notifier.c
> @@ -21,6 +21,7 @@
>   #include "util.h"
>   #include "pctl.h"
>   #include "progress.h"
> +#include "status.h"
>   
>   #ifdef CONFIG_SYSTEMD
>   #include <sys/stat.h>
> @@ -357,6 +358,18 @@ static void progress_notifier (RECOVERY_STATUS status, int event, int level, con
>   	swupdate_progress_info(status, event, msg);
>   }
>   
> +/*
> + * Status notifier: the message should be forwarded to the status
> + * interface only.
> + */
> +static void status_notifier (RECOVERY_STATUS status, int event, int level, const char *msg)
> +{
> +	if (status == PROGRESS || event == RECOVERY_DWL)
> +		return;
> +
> +	swupdate_status_info(status, event, level, msg);
> +}
> +
>   
>   #if defined(__FreeBSD__)
>   static char* socket_path = NULL;
> @@ -526,6 +539,7 @@ void notify_init(void)
>   		register_notifier(console_notifier);
>   		register_notifier(process_notifier);
>   		register_notifier(progress_notifier);
> +		register_notifier(status_notifier);
>   		start_thread(notifier_thread, NULL);
>   	}
>   }
> diff --git a/core/status_thread.c b/core/status_thread.c
> new file mode 100644
> index 0000000..e950144
> --- /dev/null
> +++ b/core/status_thread.c
> @@ -0,0 +1,176 @@
> +/*
> + * (C) Copyright 2016
> + * Stefano Babic, DENX Software Engineering, sbabic@denx.de.
> + *
> + * SPDX-License-Identifier:     GPL-2.0-only
> + */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdbool.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <sys/ioctl.h>
> +#include <fcntl.h>
> +#include <sys/types.h>
> +#include <sys/socket.h>
> +#include <sys/stat.h>
> +#include <sys/un.h>
> +#include <sys/select.h>
> +#include <arpa/inet.h>
> +#include <netinet/in.h>
> +#include <pthread.h>
> +
> +#include "swupdate.h"
> +#include <handler.h>
> +#include "util.h"
> +#include "pctl.h"
> +#include "network_ipc.h"
> +#include "network_interface.h"
> +#include <status.h>
> +#include "generated/autoconf.h"
> +
> +#ifdef CONFIG_SYSTEMD
> +#include <systemd/sd-daemon.h>
> +#endif
> +
> +struct status_conn {
> +	SIMPLEQ_ENTRY(status_conn) next;
> +	int sockfd;
> +};
> +
> +SIMPLEQ_HEAD(connections, status_conn);
> +
> +/*
> + * Structure contains data regarding
> + * current installation
> + */
> +struct swupdate_status {
> +	struct status_msg msg;
> +	char *current_image;
> +	const handler *curhnd;
> +	struct connections conns;
> +	pthread_mutex_t lock;
> +	bool step_running;
> +};
> +static struct swupdate_status notification;
> +
> +/*
> + * This must be called after acquiring the mutex
> + * for the status structure
> + */
> +static void send_status_msg(void)
> +{
> +	struct status_conn *conn, *tmp;
> +	struct swupdate_status *nmsg = &notification;
> +	void *buf;
> +	size_t count;
> +	ssize_t n;
> +
> +	SIMPLEQ_FOREACH_SAFE(conn, &nmsg->conns, next, tmp) {
> +		buf = &nmsg->msg;
> +		count = sizeof(nmsg->msg);
> +		while (count > 0) {
> +			n = send(conn->sockfd, buf, count, MSG_NOSIGNAL);
> +			if (n <= 0) {
> +				if (n == 0) {
> +					TRACE("A status client is not responding, removing it.");
> +				} else {
> +					TRACE("A status client disappeared, removing it: %s", strerror(errno));
> +				}
> +				close(conn->sockfd);
> +				SIMPLEQ_REMOVE(&nmsg->conns, conn,
> +					       	status_conn, next);
> +				free(conn);
> +				break;
> +			}
> +			count -= (size_t)n;
> +			buf = (char*)buf + n;
> +		}
> +	}
> +}
> +
> +void swupdate_status_info(RECOVERY_STATUS status, int error, int level, const char *msg)
> +{
> +	struct swupdate_status *nmsg = &notification;
> +	pthread_mutex_lock(&nmsg->lock);
> +	memset(&nmsg->msg, 0, sizeof(nmsg->msg));
> +	if (msg) {
> +		strncpy(nmsg->msg.desc, msg,
> +				sizeof(nmsg->msg.desc) - 1);
> +		clean_msg(nmsg->msg.desc, '\t');
> +		clean_msg(nmsg->msg.desc, '\n');
> +		clean_msg(nmsg->msg.desc, '\r');
> +	}
> +	nmsg->msg.current = status;
> +	nmsg->msg.level = level;
> +	nmsg->msg.error = error;
> +	nmsg->msg.desclen = strlen(nmsg->msg.desc);
> +	send_status_msg();
> +	pthread_mutex_unlock(&nmsg->lock);
> +}
> +
> +static void unlink_socket(void)
> +{
> +#ifdef CONFIG_SYSTEMD
> +	if (sd_booted() && sd_listen_fds(0) > 0) {
> +		/*
> +		 * There were socket fds handed-over by systemd,
> +		 * so don't delete the socket file.
> +		 */
> +		return;
> +	}
> +#endif
> +	unlink(get_status_socket());
> +}
> +
> +void *status_thread (void __attribute__ ((__unused__)) *data)
> +{
> +	int listen, connfd;
> +	socklen_t clilen;
> +	struct sockaddr_un cliaddr;
> +	struct swupdate_status *nmsg = &notification;
> +	struct status_conn *conn;
> +
> +	pthread_mutex_init(&nmsg->lock, NULL);
> +	SIMPLEQ_INIT(&nmsg->conns);
> +
> +	/* Initialize and bind to UDS */
> +	listen = listener_create(get_status_socket(), SOCK_STREAM);
> +	if (listen < 0 ) {
> +		ERROR("Error creating IPC socket %s, exiting.", get_status_socket());
> +		exit(2);
> +	}
> +
> +	if (atexit(unlink_socket) != 0) {
> +		TRACE("Cannot setup socket cleanup on exit, %s won't be unlinked.",
> +			get_status_socket());
> +	}
> +
> +	thread_ready();
> +	do {
> +		clilen = sizeof(cliaddr);
> +		if ( (connfd = accept(listen, (struct sockaddr *) &cliaddr, &clilen)) < 0) {
> +			if (errno == EINTR)
> +				continue;
> +			else {
> +				TRACE("Accept returns: %s", strerror(errno));
> +				continue;
> +			}
> +		}
> +
> +		/*
> +		 * Save the new connection to be handled by the status thread
> +		 */
> +		conn = (struct status_conn *)calloc(1, sizeof(*conn));
> +		if (!conn) {
> +			ERROR("Out of memory, skipping...");
> +			continue;
> +		}
> +		conn->sockfd = connfd;
> +		pthread_mutex_lock(&nmsg->lock);
> +		SIMPLEQ_INSERT_TAIL(&nmsg->conns, conn, next);
> +		pthread_mutex_unlock(&nmsg->lock);
> +	} while(1);
> +}
> diff --git a/core/swupdate.c b/core/swupdate.c
> index 949a647..fe0b92b 100644
> --- a/core/swupdate.c
> +++ b/core/swupdate.c
> @@ -43,6 +43,7 @@
>   #include "sslapi.h"
>   #include "suricatta/suricatta.h"
>   #include "progress.h"
> +#include "status.h"
>   #include "parselib.h"
>   #include "swupdate_settings.h"
>   #include "pctl.h"
> @@ -860,6 +861,8 @@ int main(int argc, char **argv)
>   
>   	start_thread(progress_bar_thread, NULL);
>   
> +	start_thread(status_thread, NULL);
> +
>   	/* wait for threads to be done before starting children */
>   	wait_threads_ready();
>   
> diff --git a/core/util.c b/core/util.c
> index 6188650..e54d156 100644
> --- a/core/util.c
> +++ b/core/util.c
> @@ -1137,3 +1137,13 @@ bool img_check_free_space(struct img_type *img, int fd)
>   
>   	return check_free_space(fd, size, img->fname);
>   }
> +
> +void clean_msg(char *msg, char drop)
> +{
> +	char *lfpos;
> +	lfpos = strchr(msg, drop);
> +	while (lfpos) {
> +		*lfpos = ' ';
> +		lfpos = strchr(msg, drop);
> +	}
> +}
> diff --git a/include/status.h b/include/status.h
> new file mode 100644
> index 0000000..a22a8f5
> --- /dev/null
> +++ b/include/status.h
> @@ -0,0 +1,23 @@
> +/*
> + * (C) Copyright 2016
> + * Stefano Babic, DENX Software Engineering, sbabic@denx.de.
> + *
> + * SPDX-License-Identifier:     GPL-2.0-only
> + */
> +
> +#ifndef _INSTALL_STATUS_H
> +#define _INSTALL_STATUS_H
> +
> +#include <swupdate_status.h>
> +#include <status_ipc.h>
> +
> +/*
> + * Internal SWUpdate functions to drive the status
> + * interface. Common status definitions for internal
> + * as well as external use are defined in status_ipc.h
> + */
> +void swupdate_status_info(RECOVERY_STATUS status, int event, int level, const char *msg);
> +
> +void *status_thread (void *data);
> +
> +#endif
> diff --git a/include/status_ipc.h b/include/status_ipc.h
> new file mode 100644
> index 0000000..fcf70d4
> --- /dev/null
> +++ b/include/status_ipc.h
> @@ -0,0 +1,53 @@
> +/*
> + * Author: Christian Storm
> + * Copyright (C) 2017, Siemens AG
> + *
> + * SPDX-License-Identifier:     LGPL-2.1-or-later
> + */
> +
> +#ifndef _STATUS_IPC_H
> +#define _STATUS_IPC_H
> +
> +#include <stdbool.h>
> +#include <swupdate_status.h>
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#define PRDESCSIZE	2048
> +
> +extern char* SOCKET_STATUS_PATH;
> +
> +/*
> + * Message sent via status socket.
> + * Data is sent in LE if required.
> + */
> +struct status_msg {
> +	unsigned int	magic;		/* Magic Number */
> +	int current;
> +	int level;
> +	int error;
> +	unsigned int 	desclen;    	/* Len of data valid in desc */
> +	char		desc[PRDESCSIZE]; /* additional information about install */
> +};
> +
> +char *get_status_socket(void);
> +
> +/* Standard function to connect to status interface */
> +int status_ipc_connect(bool reconnect);
> +
> +/*
> + * In case more as an instance of SWUpdate is running, this allows to select
> + * which should be taken
> + */
> +int status_ipc_connect_with_path(const char *socketpath, bool reconnect);
> +
> +/* Retrieve messages from status interface (it blocks) */
> +int status_ipc_receive(int *connfd, struct status_msg *msg);
> +
> +#ifdef __cplusplus
> +}   // extern "C"
> +#endif
> +
> +#endif
> diff --git a/include/util.h b/include/util.h
> index 9f29f5f..0086694 100644
> --- a/include/util.h
> +++ b/include/util.h
> @@ -220,6 +220,7 @@ int read_lines_notify(int fd, char *buf, int buf_size, int *buf_offset,
>   		      LOGLEVEL level);
>   long long get_output_size(struct img_type *img, bool strict);
>   bool img_check_free_space(struct img_type *img, int fd);
> +void clean_msg(char *msg, char drop);
>   
>   /* Decryption key functions */
>   int load_decryption_key(char *fname);
> diff --git a/ipc/Makefile b/ipc/Makefile
> index 71a1f42..661e214 100644
> --- a/ipc/Makefile
> +++ b/ipc/Makefile
> @@ -1,6 +1,6 @@
>   # Copyright (C) 2014-2018 Stefano Babic <sbabic@denx.de>
>   #
>   # SPDX-License-Identifier:     GPL-2.0-only
> -obj-y			+= network_ipc.o network_ipc-if.o progress_ipc.o
> +obj-y			+= network_ipc.o network_ipc-if.o progress_ipc.o status_ipc.o
>   
>   EXTRA_CFLAGS += -fPIC
> diff --git a/ipc/status_ipc.c b/ipc/status_ipc.c
> new file mode 100644
> index 0000000..4d95aee
> --- /dev/null
> +++ b/ipc/status_ipc.c
> @@ -0,0 +1,94 @@
> +/*
> + * Author: Christian Storm
> + * Copyright (C) 2017, Siemens AG
> + *
> + * SPDX-License-Identifier:     LGPL-2.1-or-later
> + */
> +
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +#include <errno.h>
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <stdbool.h>
> +
> +#include <status_ipc.h>
> +
> +#ifdef CONFIG_SOCKET_STATUS_PATH
> +char *SOCKET_STATUS_PATH = (char*)CONFIG_SOCKET_STATUS_PATH;
> +#else
> +char *SOCKET_STATUS_PATH = NULL;
> +#endif
> +
> +#define SOCKET_STATUS_DEFAULT  "swupdatestatus"
> +
> +char *get_status_socket(void) {
> +	if (!SOCKET_STATUS_PATH || !strlen(SOCKET_STATUS_PATH)) {
> +		const char *tmpdir = getenv("TMPDIR");
> +		if (!tmpdir)
> +			tmpdir = "/tmp";
> +
> +		if (asprintf(&SOCKET_STATUS_PATH, "%s/%s", tmpdir, SOCKET_STATUS_DEFAULT) == -1)
> +			return (char *)"/tmp/"SOCKET_STATUS_DEFAULT;
> +	}
> +
> +	return SOCKET_STATUS_PATH;
> +}
> +
> +static int _status_ipc_connect(const char *socketpath, bool reconnect)
> +{
> +	struct sockaddr_un servaddr;
> +	int fd = socket(AF_LOCAL, SOCK_STREAM, 0);
> +	bzero(&servaddr, sizeof(servaddr));
> +	servaddr.sun_family = AF_LOCAL;
> +	strncpy(servaddr.sun_path, socketpath, sizeof(servaddr.sun_path) - 1);
> +
> +	/*
> +	 * Check to get a valid socket
> +	 */
> +	if (fd < 0)
> +		return -1;
> +
> +	do {
> +		if (connect(fd, (struct sockaddr *) &servaddr, sizeof(servaddr)) == 0) {
> +			break;
> +		}
> +		if (!reconnect) {
> +			fprintf(stderr, "cannot communicate with SWUpdate via %s\n", socketpath);
> +			close(fd);
> +			return -1;
> +		}
> +
> +		usleep(10000);
> +	} while (true);
> +
> +	fprintf(stdout, "Connected to SWUpdate via %s\n", socketpath);
> +	return fd;
> +}
> +
> +int status_ipc_connect_with_path(const char *socketpath, bool reconnect) {
> +	return _status_ipc_connect(socketpath, reconnect);
> +}
> +
> +int status_ipc_connect(bool reconnect)
> +{
> +	return _status_ipc_connect(get_status_socket(), reconnect);
> +}
> +
> +int status_ipc_receive(int *connfd, struct status_msg *msg) {
> +	int ret = read(*connfd, msg, sizeof(*msg));
> +
> +	if (ret == -1 && (errno == EAGAIN || errno == EINTR))
> +		return 0;
> +
> +	if (ret != sizeof(*msg)) {
> +		fprintf(stdout, "Connection closing..\n");
> +		close(*connfd);
> +		*connfd = -1;
> +		return -1;
> +	}
> +
> +	return ret;
> +}
> diff --git a/mongoose/mongoose_interface.c b/mongoose/mongoose_interface.c
> index 2e9416b..350b06e 100644
> --- a/mongoose/mongoose_interface.c
> +++ b/mongoose/mongoose_interface.c
> @@ -25,6 +25,7 @@
>   #include <mongoose_interface.h>
>   #include <parselib.h>
>   #include <progress_ipc.h>
> +#include <status_ipc.h>
>   #include <swupdate_settings.h>
>   #include <time.h>
>   
> @@ -141,31 +142,44 @@ static void broadcast(struct mg_mgr *mgr, char *str)
>   
>   static void *broadcast_message_thread(void *data)
>   {
> +	int fd = -1;
> +
>   	for (;;) {
> -		ipc_message msg;
> -		int ret = ipc_get_status(&msg);
> +		struct status_msg msg;
> +		int ret;
>   
> -		if (!ret && strlen(msg.data.status.desc) != 0) {
> +		if (fd < 0)
> +			fd = status_ipc_connect(true);
> +		/*
> +		 * if still fails, try later
> +		 */
> +		if (fd < 0) {
> +			sleep(1);
> +			continue;
> +		}
> +
> +		ret = status_ipc_receive(&fd, &msg);
> +		if (ret != sizeof(msg))
> +			return NULL;
> +
> +		if (msg.desclen != 0) {
>   			struct mg_mgr *mgr = (struct mg_mgr *) data;
>   			char text[4096];
>   			char str[4160];
>   
> -			snescape(text, sizeof(text), msg.data.status.desc);
> +			snescape(text, sizeof(text), msg.desc);
>   
>   			snprintf(str, sizeof(str),
> -				"{\r\n"
> -				"\t\"type\": \"message\",\r\n"
> -				"\t\"level\": \"%d\",\r\n"
> -				"\t\"text\": \"%s\"\r\n"
> -				"}\r\n",
> -				(msg.data.status.error) ? 3 : 6, /* RFC 5424 */
> -				text);
> +					 "{\r\n"
> +					 "\t\"type\": \"message\",\r\n"
> +					 "\t\"level\": \"%d\",\r\n"
> +					 "\t\"text\": \"%s\"\r\n"
> +					 "}\r\n",
> +					 msg.level, /* RFC 5424 */
> +					 text);
>   
>   			broadcast(mgr, str);
> -			continue;
>   		}
> -
> -		usleep(50 * 1000);
>   	}
>   
>   	return NULL;
>

On Sun, Aug 15, 2021 at 8:35 AM Stefano Babic <sbabic@denx.de> wrote:
>
> Hi James,
>
> On 15.08.21 05:46, James Hilliard wrote:
> > Currently the only way to get status messages is by calling the
> > ctrl IPC interface, however this has the disadvantage of only
> > effectively allowing a single client at a time to read status
> > messages as they are deleted when read using this interface.
>
> This is true.
>
> >
> > In order to allow multiple clients to read status messages
> > add a dedicated interface for those messages similar to the
> > progress interface.
> >
>
> However, what I see here is that the code is copied and duplicated. I am
> also concerning about creating an additional socket interface, when
> there is already two sockets, one to control SWUpdate (and send a SWU),
> one to get information about the update. The additional status socker is
> mostly a copy frim the progress and it is also unidirectional, that mean
> clients just want to get informed and they do not send data back. More
> or less, the limits of GET_STATUS led me to create the progress
> interface. The GET_STATUS works also in polling mode, while the progress
> interface sends events to the listen. For external interface, a syslog
> notifier sends the same data to the syslog socket.
>
> If this is really required, I do not like to get duplicated code. It
> should be solved reusing in some way the progress interface (and taking
> care of some compatibility issues that could be raised).

Should I just reduce the duplication of the code by refactoring it to share more
or should I reuse the progress socket as well by implementing it as part of
core/progress_thread.c? I mostly kept it separate since the message format
for that seemed somewhat specific to the progress updates.

>
> Best regards,
> Stefano
>
> > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> > ---
> >   Kconfig                       |   5 +
> >   core/Makefile                 |   1 +
> >   core/network_thread.c         |  10 --
> >   core/notifier.c               |  14 +++
> >   core/status_thread.c          | 176 ++++++++++++++++++++++++++++++++++
> >   core/swupdate.c               |   3 +
> >   core/util.c                   |  10 ++
> >   include/status.h              |  23 +++++
> >   include/status_ipc.h          |  53 ++++++++++
> >   include/util.h                |   1 +
> >   ipc/Makefile                  |   2 +-
> >   ipc/status_ipc.c              |  94 ++++++++++++++++++
> >   mongoose/mongoose_interface.c |  42 +++++---
> >   13 files changed, 409 insertions(+), 25 deletions(-)
> >   create mode 100644 core/status_thread.c
> >   create mode 100644 include/status.h
> >   create mode 100644 include/status_ipc.h
> >   create mode 100644 ipc/status_ipc.c
> >
> > diff --git a/Kconfig b/Kconfig
> > index dc86957..7fc7382 100644
> > --- a/Kconfig
> > +++ b/Kconfig
> > @@ -215,6 +215,11 @@ config SOCKET_PROGRESS_PATH
> >       help
> >         Path to the socket progress information is sent to.
> >
> > +config SOCKET_STATUS_PATH
> > +     string "SWUpdate status socket path"
> > +     help
> > +       Path to the socket status information is sent to.
> > +
> >   config SOCKET_NOTIFIER_DIRECTORY
> >       string "SWUpdate notifier socket directory"
> >       depends on HAVE_FREEBSD
> > diff --git a/core/Makefile b/core/Makefile
> > index fa30e6e..e6aeba4 100644
> > --- a/core/Makefile
> > +++ b/core/Makefile
> > @@ -22,6 +22,7 @@ obj-y += swupdate.o \
> >        network_thread.o \
> >        stream_interface.o \
> >        progress_thread.o \
> > +      status_thread.o \
> >        parsing_library.o \
> >        artifacts_versions.o \
> >        swupdate_dict.o \
> > diff --git a/core/network_thread.c b/core/network_thread.c
> > index adaf21c..59f1c15 100644
> > --- a/core/network_thread.c
> > +++ b/core/network_thread.c
> > @@ -106,16 +106,6 @@ static bool is_selection_allowed(const char *software_set, char *running_mode,
> >       return allowed;
> >   }
> >
> > -static void clean_msg(char *msg, char drop)
> > -{
> > -     char *lfpos;
> > -     lfpos = strchr(msg, drop);
> > -     while (lfpos) {
> > -             *lfpos = ' ';
> > -             lfpos = strchr(msg, drop);
> > -     }
> > -}
> > -
> >   static void network_notifier(RECOVERY_STATUS status, int error, int level, const char *msg)
> >   {
> >       int len = msg ? strlen(msg) : 0;
> > diff --git a/core/notifier.c b/core/notifier.c
> > index 810769c..7510f93 100644
> > --- a/core/notifier.c
> > +++ b/core/notifier.c
> > @@ -21,6 +21,7 @@
> >   #include "util.h"
> >   #include "pctl.h"
> >   #include "progress.h"
> > +#include "status.h"
> >
> >   #ifdef CONFIG_SYSTEMD
> >   #include <sys/stat.h>
> > @@ -357,6 +358,18 @@ static void progress_notifier (RECOVERY_STATUS status, int event, int level, con
> >       swupdate_progress_info(status, event, msg);
> >   }
> >
> > +/*
> > + * Status notifier: the message should be forwarded to the status
> > + * interface only.
> > + */
> > +static void status_notifier (RECOVERY_STATUS status, int event, int level, const char *msg)
> > +{
> > +     if (status == PROGRESS || event == RECOVERY_DWL)
> > +             return;
> > +
> > +     swupdate_status_info(status, event, level, msg);
> > +}
> > +
> >
> >   #if defined(__FreeBSD__)
> >   static char* socket_path = NULL;
> > @@ -526,6 +539,7 @@ void notify_init(void)
> >               register_notifier(console_notifier);
> >               register_notifier(process_notifier);
> >               register_notifier(progress_notifier);
> > +             register_notifier(status_notifier);
> >               start_thread(notifier_thread, NULL);
> >       }
> >   }
> > diff --git a/core/status_thread.c b/core/status_thread.c
> > new file mode 100644
> > index 0000000..e950144
> > --- /dev/null
> > +++ b/core/status_thread.c
> > @@ -0,0 +1,176 @@
> > +/*
> > + * (C) Copyright 2016
> > + * Stefano Babic, DENX Software Engineering, sbabic@denx.de.
> > + *
> > + * SPDX-License-Identifier:     GPL-2.0-only
> > + */
> > +
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <stdbool.h>
> > +#include <unistd.h>
> > +#include <string.h>
> > +#include <errno.h>
> > +#include <sys/ioctl.h>
> > +#include <fcntl.h>
> > +#include <sys/types.h>
> > +#include <sys/socket.h>
> > +#include <sys/stat.h>
> > +#include <sys/un.h>
> > +#include <sys/select.h>
> > +#include <arpa/inet.h>
> > +#include <netinet/in.h>
> > +#include <pthread.h>
> > +
> > +#include "swupdate.h"
> > +#include <handler.h>
> > +#include "util.h"
> > +#include "pctl.h"
> > +#include "network_ipc.h"
> > +#include "network_interface.h"
> > +#include <status.h>
> > +#include "generated/autoconf.h"
> > +
> > +#ifdef CONFIG_SYSTEMD
> > +#include <systemd/sd-daemon.h>
> > +#endif
> > +
> > +struct status_conn {
> > +     SIMPLEQ_ENTRY(status_conn) next;
> > +     int sockfd;
> > +};
> > +
> > +SIMPLEQ_HEAD(connections, status_conn);
> > +
> > +/*
> > + * Structure contains data regarding
> > + * current installation
> > + */
> > +struct swupdate_status {
> > +     struct status_msg msg;
> > +     char *current_image;
> > +     const handler *curhnd;
> > +     struct connections conns;
> > +     pthread_mutex_t lock;
> > +     bool step_running;
> > +};
> > +static struct swupdate_status notification;
> > +
> > +/*
> > + * This must be called after acquiring the mutex
> > + * for the status structure
> > + */
> > +static void send_status_msg(void)
> > +{
> > +     struct status_conn *conn, *tmp;
> > +     struct swupdate_status *nmsg = &notification;
> > +     void *buf;
> > +     size_t count;
> > +     ssize_t n;
> > +
> > +     SIMPLEQ_FOREACH_SAFE(conn, &nmsg->conns, next, tmp) {
> > +             buf = &nmsg->msg;
> > +             count = sizeof(nmsg->msg);
> > +             while (count > 0) {
> > +                     n = send(conn->sockfd, buf, count, MSG_NOSIGNAL);
> > +                     if (n <= 0) {
> > +                             if (n == 0) {
> > +                                     TRACE("A status client is not responding, removing it.");
> > +                             } else {
> > +                                     TRACE("A status client disappeared, removing it: %s", strerror(errno));
> > +                             }
> > +                             close(conn->sockfd);
> > +                             SIMPLEQ_REMOVE(&nmsg->conns, conn,
> > +                                             status_conn, next);
> > +                             free(conn);
> > +                             break;
> > +                     }
> > +                     count -= (size_t)n;
> > +                     buf = (char*)buf + n;
> > +             }
> > +     }
> > +}
> > +
> > +void swupdate_status_info(RECOVERY_STATUS status, int error, int level, const char *msg)
> > +{
> > +     struct swupdate_status *nmsg = &notification;
> > +     pthread_mutex_lock(&nmsg->lock);
> > +     memset(&nmsg->msg, 0, sizeof(nmsg->msg));
> > +     if (msg) {
> > +             strncpy(nmsg->msg.desc, msg,
> > +                             sizeof(nmsg->msg.desc) - 1);
> > +             clean_msg(nmsg->msg.desc, '\t');
> > +             clean_msg(nmsg->msg.desc, '\n');
> > +             clean_msg(nmsg->msg.desc, '\r');
> > +     }
> > +     nmsg->msg.current = status;
> > +     nmsg->msg.level = level;
> > +     nmsg->msg.error = error;
> > +     nmsg->msg.desclen = strlen(nmsg->msg.desc);
> > +     send_status_msg();
> > +     pthread_mutex_unlock(&nmsg->lock);
> > +}
> > +
> > +static void unlink_socket(void)
> > +{
> > +#ifdef CONFIG_SYSTEMD
> > +     if (sd_booted() && sd_listen_fds(0) > 0) {
> > +             /*
> > +              * There were socket fds handed-over by systemd,
> > +              * so don't delete the socket file.
> > +              */
> > +             return;
> > +     }
> > +#endif
> > +     unlink(get_status_socket());
> > +}
> > +
> > +void *status_thread (void __attribute__ ((__unused__)) *data)
> > +{
> > +     int listen, connfd;
> > +     socklen_t clilen;
> > +     struct sockaddr_un cliaddr;
> > +     struct swupdate_status *nmsg = &notification;
> > +     struct status_conn *conn;
> > +
> > +     pthread_mutex_init(&nmsg->lock, NULL);
> > +     SIMPLEQ_INIT(&nmsg->conns);
> > +
> > +     /* Initialize and bind to UDS */
> > +     listen = listener_create(get_status_socket(), SOCK_STREAM);
> > +     if (listen < 0 ) {
> > +             ERROR("Error creating IPC socket %s, exiting.", get_status_socket());
> > +             exit(2);
> > +     }
> > +
> > +     if (atexit(unlink_socket) != 0) {
> > +             TRACE("Cannot setup socket cleanup on exit, %s won't be unlinked.",
> > +                     get_status_socket());
> > +     }
> > +
> > +     thread_ready();
> > +     do {
> > +             clilen = sizeof(cliaddr);
> > +             if ( (connfd = accept(listen, (struct sockaddr *) &cliaddr, &clilen)) < 0) {
> > +                     if (errno == EINTR)
> > +                             continue;
> > +                     else {
> > +                             TRACE("Accept returns: %s", strerror(errno));
> > +                             continue;
> > +                     }
> > +             }
> > +
> > +             /*
> > +              * Save the new connection to be handled by the status thread
> > +              */
> > +             conn = (struct status_conn *)calloc(1, sizeof(*conn));
> > +             if (!conn) {
> > +                     ERROR("Out of memory, skipping...");
> > +                     continue;
> > +             }
> > +             conn->sockfd = connfd;
> > +             pthread_mutex_lock(&nmsg->lock);
> > +             SIMPLEQ_INSERT_TAIL(&nmsg->conns, conn, next);
> > +             pthread_mutex_unlock(&nmsg->lock);
> > +     } while(1);
> > +}
> > diff --git a/core/swupdate.c b/core/swupdate.c
> > index 949a647..fe0b92b 100644
> > --- a/core/swupdate.c
> > +++ b/core/swupdate.c
> > @@ -43,6 +43,7 @@
> >   #include "sslapi.h"
> >   #include "suricatta/suricatta.h"
> >   #include "progress.h"
> > +#include "status.h"
> >   #include "parselib.h"
> >   #include "swupdate_settings.h"
> >   #include "pctl.h"
> > @@ -860,6 +861,8 @@ int main(int argc, char **argv)
> >
> >       start_thread(progress_bar_thread, NULL);
> >
> > +     start_thread(status_thread, NULL);
> > +
> >       /* wait for threads to be done before starting children */
> >       wait_threads_ready();
> >
> > diff --git a/core/util.c b/core/util.c
> > index 6188650..e54d156 100644
> > --- a/core/util.c
> > +++ b/core/util.c
> > @@ -1137,3 +1137,13 @@ bool img_check_free_space(struct img_type *img, int fd)
> >
> >       return check_free_space(fd, size, img->fname);
> >   }
> > +
> > +void clean_msg(char *msg, char drop)
> > +{
> > +     char *lfpos;
> > +     lfpos = strchr(msg, drop);
> > +     while (lfpos) {
> > +             *lfpos = ' ';
> > +             lfpos = strchr(msg, drop);
> > +     }
> > +}
> > diff --git a/include/status.h b/include/status.h
> > new file mode 100644
> > index 0000000..a22a8f5
> > --- /dev/null
> > +++ b/include/status.h
> > @@ -0,0 +1,23 @@
> > +/*
> > + * (C) Copyright 2016
> > + * Stefano Babic, DENX Software Engineering, sbabic@denx.de.
> > + *
> > + * SPDX-License-Identifier:     GPL-2.0-only
> > + */
> > +
> > +#ifndef _INSTALL_STATUS_H
> > +#define _INSTALL_STATUS_H
> > +
> > +#include <swupdate_status.h>
> > +#include <status_ipc.h>
> > +
> > +/*
> > + * Internal SWUpdate functions to drive the status
> > + * interface. Common status definitions for internal
> > + * as well as external use are defined in status_ipc.h
> > + */
> > +void swupdate_status_info(RECOVERY_STATUS status, int event, int level, const char *msg);
> > +
> > +void *status_thread (void *data);
> > +
> > +#endif
> > diff --git a/include/status_ipc.h b/include/status_ipc.h
> > new file mode 100644
> > index 0000000..fcf70d4
> > --- /dev/null
> > +++ b/include/status_ipc.h
> > @@ -0,0 +1,53 @@
> > +/*
> > + * Author: Christian Storm
> > + * Copyright (C) 2017, Siemens AG
> > + *
> > + * SPDX-License-Identifier:     LGPL-2.1-or-later
> > + */
> > +
> > +#ifndef _STATUS_IPC_H
> > +#define _STATUS_IPC_H
> > +
> > +#include <stdbool.h>
> > +#include <swupdate_status.h>
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +#define PRDESCSIZE   2048
> > +
> > +extern char* SOCKET_STATUS_PATH;
> > +
> > +/*
> > + * Message sent via status socket.
> > + * Data is sent in LE if required.
> > + */
> > +struct status_msg {
> > +     unsigned int    magic;          /* Magic Number */
> > +     int current;
> > +     int level;
> > +     int error;
> > +     unsigned int    desclen;        /* Len of data valid in desc */
> > +     char            desc[PRDESCSIZE]; /* additional information about install */
> > +};
> > +
> > +char *get_status_socket(void);
> > +
> > +/* Standard function to connect to status interface */
> > +int status_ipc_connect(bool reconnect);
> > +
> > +/*
> > + * In case more as an instance of SWUpdate is running, this allows to select
> > + * which should be taken
> > + */
> > +int status_ipc_connect_with_path(const char *socketpath, bool reconnect);
> > +
> > +/* Retrieve messages from status interface (it blocks) */
> > +int status_ipc_receive(int *connfd, struct status_msg *msg);
> > +
> > +#ifdef __cplusplus
> > +}   // extern "C"
> > +#endif
> > +
> > +#endif
> > diff --git a/include/util.h b/include/util.h
> > index 9f29f5f..0086694 100644
> > --- a/include/util.h
> > +++ b/include/util.h
> > @@ -220,6 +220,7 @@ int read_lines_notify(int fd, char *buf, int buf_size, int *buf_offset,
> >                     LOGLEVEL level);
> >   long long get_output_size(struct img_type *img, bool strict);
> >   bool img_check_free_space(struct img_type *img, int fd);
> > +void clean_msg(char *msg, char drop);
> >
> >   /* Decryption key functions */
> >   int load_decryption_key(char *fname);
> > diff --git a/ipc/Makefile b/ipc/Makefile
> > index 71a1f42..661e214 100644
> > --- a/ipc/Makefile
> > +++ b/ipc/Makefile
> > @@ -1,6 +1,6 @@
> >   # Copyright (C) 2014-2018 Stefano Babic <sbabic@denx.de>
> >   #
> >   # SPDX-License-Identifier:     GPL-2.0-only
> > -obj-y                        += network_ipc.o network_ipc-if.o progress_ipc.o
> > +obj-y                        += network_ipc.o network_ipc-if.o progress_ipc.o status_ipc.o
> >
> >   EXTRA_CFLAGS += -fPIC
> > diff --git a/ipc/status_ipc.c b/ipc/status_ipc.c
> > new file mode 100644
> > index 0000000..4d95aee
> > --- /dev/null
> > +++ b/ipc/status_ipc.c
> > @@ -0,0 +1,94 @@
> > +/*
> > + * Author: Christian Storm
> > + * Copyright (C) 2017, Siemens AG
> > + *
> > + * SPDX-License-Identifier:     LGPL-2.1-or-later
> > + */
> > +
> > +#include <sys/socket.h>
> > +#include <sys/un.h>
> > +#include <errno.h>
> > +#include <string.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +#include <stdbool.h>
> > +
> > +#include <status_ipc.h>
> > +
> > +#ifdef CONFIG_SOCKET_STATUS_PATH
> > +char *SOCKET_STATUS_PATH = (char*)CONFIG_SOCKET_STATUS_PATH;
> > +#else
> > +char *SOCKET_STATUS_PATH = NULL;
> > +#endif
> > +
> > +#define SOCKET_STATUS_DEFAULT  "swupdatestatus"
> > +
> > +char *get_status_socket(void) {
> > +     if (!SOCKET_STATUS_PATH || !strlen(SOCKET_STATUS_PATH)) {
> > +             const char *tmpdir = getenv("TMPDIR");
> > +             if (!tmpdir)
> > +                     tmpdir = "/tmp";
> > +
> > +             if (asprintf(&SOCKET_STATUS_PATH, "%s/%s", tmpdir, SOCKET_STATUS_DEFAULT) == -1)
> > +                     return (char *)"/tmp/"SOCKET_STATUS_DEFAULT;
> > +     }
> > +
> > +     return SOCKET_STATUS_PATH;
> > +}
> > +
> > +static int _status_ipc_connect(const char *socketpath, bool reconnect)
> > +{
> > +     struct sockaddr_un servaddr;
> > +     int fd = socket(AF_LOCAL, SOCK_STREAM, 0);
> > +     bzero(&servaddr, sizeof(servaddr));
> > +     servaddr.sun_family = AF_LOCAL;
> > +     strncpy(servaddr.sun_path, socketpath, sizeof(servaddr.sun_path) - 1);
> > +
> > +     /*
> > +      * Check to get a valid socket
> > +      */
> > +     if (fd < 0)
> > +             return -1;
> > +
> > +     do {
> > +             if (connect(fd, (struct sockaddr *) &servaddr, sizeof(servaddr)) == 0) {
> > +                     break;
> > +             }
> > +             if (!reconnect) {
> > +                     fprintf(stderr, "cannot communicate with SWUpdate via %s\n", socketpath);
> > +                     close(fd);
> > +                     return -1;
> > +             }
> > +
> > +             usleep(10000);
> > +     } while (true);
> > +
> > +     fprintf(stdout, "Connected to SWUpdate via %s\n", socketpath);
> > +     return fd;
> > +}
> > +
> > +int status_ipc_connect_with_path(const char *socketpath, bool reconnect) {
> > +     return _status_ipc_connect(socketpath, reconnect);
> > +}
> > +
> > +int status_ipc_connect(bool reconnect)
> > +{
> > +     return _status_ipc_connect(get_status_socket(), reconnect);
> > +}
> > +
> > +int status_ipc_receive(int *connfd, struct status_msg *msg) {
> > +     int ret = read(*connfd, msg, sizeof(*msg));
> > +
> > +     if (ret == -1 && (errno == EAGAIN || errno == EINTR))
> > +             return 0;
> > +
> > +     if (ret != sizeof(*msg)) {
> > +             fprintf(stdout, "Connection closing..\n");
> > +             close(*connfd);
> > +             *connfd = -1;
> > +             return -1;
> > +     }
> > +
> > +     return ret;
> > +}
> > diff --git a/mongoose/mongoose_interface.c b/mongoose/mongoose_interface.c
> > index 2e9416b..350b06e 100644
> > --- a/mongoose/mongoose_interface.c
> > +++ b/mongoose/mongoose_interface.c
> > @@ -25,6 +25,7 @@
> >   #include <mongoose_interface.h>
> >   #include <parselib.h>
> >   #include <progress_ipc.h>
> > +#include <status_ipc.h>
> >   #include <swupdate_settings.h>
> >   #include <time.h>
> >
> > @@ -141,31 +142,44 @@ static void broadcast(struct mg_mgr *mgr, char *str)
> >
> >   static void *broadcast_message_thread(void *data)
> >   {
> > +     int fd = -1;
> > +
> >       for (;;) {
> > -             ipc_message msg;
> > -             int ret = ipc_get_status(&msg);
> > +             struct status_msg msg;
> > +             int ret;
> >
> > -             if (!ret && strlen(msg.data.status.desc) != 0) {
> > +             if (fd < 0)
> > +                     fd = status_ipc_connect(true);
> > +             /*
> > +              * if still fails, try later
> > +              */
> > +             if (fd < 0) {
> > +                     sleep(1);
> > +                     continue;
> > +             }
> > +
> > +             ret = status_ipc_receive(&fd, &msg);
> > +             if (ret != sizeof(msg))
> > +                     return NULL;
> > +
> > +             if (msg.desclen != 0) {
> >                       struct mg_mgr *mgr = (struct mg_mgr *) data;
> >                       char text[4096];
> >                       char str[4160];
> >
> > -                     snescape(text, sizeof(text), msg.data.status.desc);
> > +                     snescape(text, sizeof(text), msg.desc);
> >
> >                       snprintf(str, sizeof(str),
> > -                             "{\r\n"
> > -                             "\t\"type\": \"message\",\r\n"
> > -                             "\t\"level\": \"%d\",\r\n"
> > -                             "\t\"text\": \"%s\"\r\n"
> > -                             "}\r\n",
> > -                             (msg.data.status.error) ? 3 : 6, /* RFC 5424 */
> > -                             text);
> > +                                      "{\r\n"
> > +                                      "\t\"type\": \"message\",\r\n"
> > +                                      "\t\"level\": \"%d\",\r\n"
> > +                                      "\t\"text\": \"%s\"\r\n"
> > +                                      "}\r\n",
> > +                                      msg.level, /* RFC 5424 */
> > +                                      text);
> >
> >                       broadcast(mgr, str);
> > -                     continue;
> >               }
> > -
> > -             usleep(50 * 1000);
> >       }
> >
> >       return NULL;
> >
>
> --
> =====================================================================
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de
> =====================================================================

On Sun, Aug 15, 2021 at 8:35 AM Stefano Babic <sbabic@denx.de> wrote:
>
> Hi James,
>
> On 15.08.21 05:46, James Hilliard wrote:
> > Currently the only way to get status messages is by calling the
> > ctrl IPC interface, however this has the disadvantage of only
> > effectively allowing a single client at a time to read status
> > messages as they are deleted when read using this interface.
>
> This is true.
>
> >
> > In order to allow multiple clients to read status messages
> > add a dedicated interface for those messages similar to the
> > progress interface.
> >
>
> However, what I see here is that the code is copied and duplicated. I am
> also concerning about creating an additional socket interface, when
> there is already two sockets, one to control SWUpdate (and send a SWU),
> one to get information about the update. The additional status socker is
> mostly a copy frim the progress and it is also unidirectional, that mean
> clients just want to get informed and they do not send data back. More
> or less, the limits of GET_STATUS led me to create the progress
> interface. The GET_STATUS works also in polling mode, while the progress
> interface sends events to the listen. For external interface, a syslog
> notifier sends the same data to the syslog socket.
>
> If this is really required, I do not like to get duplicated code. It
> should be solved reusing in some way the progress interface (and taking
> care of some compatibility issues that could be raised).

Extending the progress interface seemed somewhat problematic for backwards
compatibility, at least I didn't see a good way to handle that,
however I think I
managed to get it working reasonably well by extending the main network thread
here:
https://groups.google.com/g/swupdate/c/eqzLb4YKY7A

Does this approach look ok to you? One reason I went with this approach is that
the msgdata response structure format is reusable for these
notifications as it is
already the right size.

I also wanted to make sure that clients wouldn't lose messages if they connected
slightly late so I made it so that this interface does not purge
entries from the
notifymsgs buffer unless GET_STATUS is called(I kept existing behavior with this
for backwards compatibility).

>
> Best regards,
> Stefano
>
> > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> > ---
> >   Kconfig                       |   5 +
> >   core/Makefile                 |   1 +
> >   core/network_thread.c         |  10 --
> >   core/notifier.c               |  14 +++
> >   core/status_thread.c          | 176 ++++++++++++++++++++++++++++++++++
> >   core/swupdate.c               |   3 +
> >   core/util.c                   |  10 ++
> >   include/status.h              |  23 +++++
> >   include/status_ipc.h          |  53 ++++++++++
> >   include/util.h                |   1 +
> >   ipc/Makefile                  |   2 +-
> >   ipc/status_ipc.c              |  94 ++++++++++++++++++
> >   mongoose/mongoose_interface.c |  42 +++++---
> >   13 files changed, 409 insertions(+), 25 deletions(-)
> >   create mode 100644 core/status_thread.c
> >   create mode 100644 include/status.h
> >   create mode 100644 include/status_ipc.h
> >   create mode 100644 ipc/status_ipc.c
> >
> > diff --git a/Kconfig b/Kconfig
> > index dc86957..7fc7382 100644
> > --- a/Kconfig
> > +++ b/Kconfig
> > @@ -215,6 +215,11 @@ config SOCKET_PROGRESS_PATH
> >       help
> >         Path to the socket progress information is sent to.
> >
> > +config SOCKET_STATUS_PATH
> > +     string "SWUpdate status socket path"
> > +     help
> > +       Path to the socket status information is sent to.
> > +
> >   config SOCKET_NOTIFIER_DIRECTORY
> >       string "SWUpdate notifier socket directory"
> >       depends on HAVE_FREEBSD
> > diff --git a/core/Makefile b/core/Makefile
> > index fa30e6e..e6aeba4 100644
> > --- a/core/Makefile
> > +++ b/core/Makefile
> > @@ -22,6 +22,7 @@ obj-y += swupdate.o \
> >        network_thread.o \
> >        stream_interface.o \
> >        progress_thread.o \
> > +      status_thread.o \
> >        parsing_library.o \
> >        artifacts_versions.o \
> >        swupdate_dict.o \
> > diff --git a/core/network_thread.c b/core/network_thread.c
> > index adaf21c..59f1c15 100644
> > --- a/core/network_thread.c
> > +++ b/core/network_thread.c
> > @@ -106,16 +106,6 @@ static bool is_selection_allowed(const char *software_set, char *running_mode,
> >       return allowed;
> >   }
> >
> > -static void clean_msg(char *msg, char drop)
> > -{
> > -     char *lfpos;
> > -     lfpos = strchr(msg, drop);
> > -     while (lfpos) {
> > -             *lfpos = ' ';
> > -             lfpos = strchr(msg, drop);
> > -     }
> > -}
> > -
> >   static void network_notifier(RECOVERY_STATUS status, int error, int level, const char *msg)
> >   {
> >       int len = msg ? strlen(msg) : 0;
> > diff --git a/core/notifier.c b/core/notifier.c
> > index 810769c..7510f93 100644
> > --- a/core/notifier.c
> > +++ b/core/notifier.c
> > @@ -21,6 +21,7 @@
> >   #include "util.h"
> >   #include "pctl.h"
> >   #include "progress.h"
> > +#include "status.h"
> >
> >   #ifdef CONFIG_SYSTEMD
> >   #include <sys/stat.h>
> > @@ -357,6 +358,18 @@ static void progress_notifier (RECOVERY_STATUS status, int event, int level, con
> >       swupdate_progress_info(status, event, msg);
> >   }
> >
> > +/*
> > + * Status notifier: the message should be forwarded to the status
> > + * interface only.
> > + */
> > +static void status_notifier (RECOVERY_STATUS status, int event, int level, const char *msg)
> > +{
> > +     if (status == PROGRESS || event == RECOVERY_DWL)
> > +             return;
> > +
> > +     swupdate_status_info(status, event, level, msg);
> > +}
> > +
> >
> >   #if defined(__FreeBSD__)
> >   static char* socket_path = NULL;
> > @@ -526,6 +539,7 @@ void notify_init(void)
> >               register_notifier(console_notifier);
> >               register_notifier(process_notifier);
> >               register_notifier(progress_notifier);
> > +             register_notifier(status_notifier);
> >               start_thread(notifier_thread, NULL);
> >       }
> >   }
> > diff --git a/core/status_thread.c b/core/status_thread.c
> > new file mode 100644
> > index 0000000..e950144
> > --- /dev/null
> > +++ b/core/status_thread.c
> > @@ -0,0 +1,176 @@
> > +/*
> > + * (C) Copyright 2016
> > + * Stefano Babic, DENX Software Engineering, sbabic@denx.de.
> > + *
> > + * SPDX-License-Identifier:     GPL-2.0-only
> > + */
> > +
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <stdbool.h>
> > +#include <unistd.h>
> > +#include <string.h>
> > +#include <errno.h>
> > +#include <sys/ioctl.h>
> > +#include <fcntl.h>
> > +#include <sys/types.h>
> > +#include <sys/socket.h>
> > +#include <sys/stat.h>
> > +#include <sys/un.h>
> > +#include <sys/select.h>
> > +#include <arpa/inet.h>
> > +#include <netinet/in.h>
> > +#include <pthread.h>
> > +
> > +#include "swupdate.h"
> > +#include <handler.h>
> > +#include "util.h"
> > +#include "pctl.h"
> > +#include "network_ipc.h"
> > +#include "network_interface.h"
> > +#include <status.h>
> > +#include "generated/autoconf.h"
> > +
> > +#ifdef CONFIG_SYSTEMD
> > +#include <systemd/sd-daemon.h>
> > +#endif
> > +
> > +struct status_conn {
> > +     SIMPLEQ_ENTRY(status_conn) next;
> > +     int sockfd;
> > +};
> > +
> > +SIMPLEQ_HEAD(connections, status_conn);
> > +
> > +/*
> > + * Structure contains data regarding
> > + * current installation
> > + */
> > +struct swupdate_status {
> > +     struct status_msg msg;
> > +     char *current_image;
> > +     const handler *curhnd;
> > +     struct connections conns;
> > +     pthread_mutex_t lock;
> > +     bool step_running;
> > +};
> > +static struct swupdate_status notification;
> > +
> > +/*
> > + * This must be called after acquiring the mutex
> > + * for the status structure
> > + */
> > +static void send_status_msg(void)
> > +{
> > +     struct status_conn *conn, *tmp;
> > +     struct swupdate_status *nmsg = &notification;
> > +     void *buf;
> > +     size_t count;
> > +     ssize_t n;
> > +
> > +     SIMPLEQ_FOREACH_SAFE(conn, &nmsg->conns, next, tmp) {
> > +             buf = &nmsg->msg;
> > +             count = sizeof(nmsg->msg);
> > +             while (count > 0) {
> > +                     n = send(conn->sockfd, buf, count, MSG_NOSIGNAL);
> > +                     if (n <= 0) {
> > +                             if (n == 0) {
> > +                                     TRACE("A status client is not responding, removing it.");
> > +                             } else {
> > +                                     TRACE("A status client disappeared, removing it: %s", strerror(errno));
> > +                             }
> > +                             close(conn->sockfd);
> > +                             SIMPLEQ_REMOVE(&nmsg->conns, conn,
> > +                                             status_conn, next);
> > +                             free(conn);
> > +                             break;
> > +                     }
> > +                     count -= (size_t)n;
> > +                     buf = (char*)buf + n;
> > +             }
> > +     }
> > +}
> > +
> > +void swupdate_status_info(RECOVERY_STATUS status, int error, int level, const char *msg)
> > +{
> > +     struct swupdate_status *nmsg = &notification;
> > +     pthread_mutex_lock(&nmsg->lock);
> > +     memset(&nmsg->msg, 0, sizeof(nmsg->msg));
> > +     if (msg) {
> > +             strncpy(nmsg->msg.desc, msg,
> > +                             sizeof(nmsg->msg.desc) - 1);
> > +             clean_msg(nmsg->msg.desc, '\t');
> > +             clean_msg(nmsg->msg.desc, '\n');
> > +             clean_msg(nmsg->msg.desc, '\r');
> > +     }
> > +     nmsg->msg.current = status;
> > +     nmsg->msg.level = level;
> > +     nmsg->msg.error = error;
> > +     nmsg->msg.desclen = strlen(nmsg->msg.desc);
> > +     send_status_msg();
> > +     pthread_mutex_unlock(&nmsg->lock);
> > +}
> > +
> > +static void unlink_socket(void)
> > +{
> > +#ifdef CONFIG_SYSTEMD
> > +     if (sd_booted() && sd_listen_fds(0) > 0) {
> > +             /*
> > +              * There were socket fds handed-over by systemd,
> > +              * so don't delete the socket file.
> > +              */
> > +             return;
> > +     }
> > +#endif
> > +     unlink(get_status_socket());
> > +}
> > +
> > +void *status_thread (void __attribute__ ((__unused__)) *data)
> > +{
> > +     int listen, connfd;
> > +     socklen_t clilen;
> > +     struct sockaddr_un cliaddr;
> > +     struct swupdate_status *nmsg = &notification;
> > +     struct status_conn *conn;
> > +
> > +     pthread_mutex_init(&nmsg->lock, NULL);
> > +     SIMPLEQ_INIT(&nmsg->conns);
> > +
> > +     /* Initialize and bind to UDS */
> > +     listen = listener_create(get_status_socket(), SOCK_STREAM);
> > +     if (listen < 0 ) {
> > +             ERROR("Error creating IPC socket %s, exiting.", get_status_socket());
> > +             exit(2);
> > +     }
> > +
> > +     if (atexit(unlink_socket) != 0) {
> > +             TRACE("Cannot setup socket cleanup on exit, %s won't be unlinked.",
> > +                     get_status_socket());
> > +     }
> > +
> > +     thread_ready();
> > +     do {
> > +             clilen = sizeof(cliaddr);
> > +             if ( (connfd = accept(listen, (struct sockaddr *) &cliaddr, &clilen)) < 0) {
> > +                     if (errno == EINTR)
> > +                             continue;
> > +                     else {
> > +                             TRACE("Accept returns: %s", strerror(errno));
> > +                             continue;
> > +                     }
> > +             }
> > +
> > +             /*
> > +              * Save the new connection to be handled by the status thread
> > +              */
> > +             conn = (struct status_conn *)calloc(1, sizeof(*conn));
> > +             if (!conn) {
> > +                     ERROR("Out of memory, skipping...");
> > +                     continue;
> > +             }
> > +             conn->sockfd = connfd;
> > +             pthread_mutex_lock(&nmsg->lock);
> > +             SIMPLEQ_INSERT_TAIL(&nmsg->conns, conn, next);
> > +             pthread_mutex_unlock(&nmsg->lock);
> > +     } while(1);
> > +}
> > diff --git a/core/swupdate.c b/core/swupdate.c
> > index 949a647..fe0b92b 100644
> > --- a/core/swupdate.c
> > +++ b/core/swupdate.c
> > @@ -43,6 +43,7 @@
> >   #include "sslapi.h"
> >   #include "suricatta/suricatta.h"
> >   #include "progress.h"
> > +#include "status.h"
> >   #include "parselib.h"
> >   #include "swupdate_settings.h"
> >   #include "pctl.h"
> > @@ -860,6 +861,8 @@ int main(int argc, char **argv)
> >
> >       start_thread(progress_bar_thread, NULL);
> >
> > +     start_thread(status_thread, NULL);
> > +
> >       /* wait for threads to be done before starting children */
> >       wait_threads_ready();
> >
> > diff --git a/core/util.c b/core/util.c
> > index 6188650..e54d156 100644
> > --- a/core/util.c
> > +++ b/core/util.c
> > @@ -1137,3 +1137,13 @@ bool img_check_free_space(struct img_type *img, int fd)
> >
> >       return check_free_space(fd, size, img->fname);
> >   }
> > +
> > +void clean_msg(char *msg, char drop)
> > +{
> > +     char *lfpos;
> > +     lfpos = strchr(msg, drop);
> > +     while (lfpos) {
> > +             *lfpos = ' ';
> > +             lfpos = strchr(msg, drop);
> > +     }
> > +}
> > diff --git a/include/status.h b/include/status.h
> > new file mode 100644
> > index 0000000..a22a8f5
> > --- /dev/null
> > +++ b/include/status.h
> > @@ -0,0 +1,23 @@
> > +/*
> > + * (C) Copyright 2016
> > + * Stefano Babic, DENX Software Engineering, sbabic@denx.de.
> > + *
> > + * SPDX-License-Identifier:     GPL-2.0-only
> > + */
> > +
> > +#ifndef _INSTALL_STATUS_H
> > +#define _INSTALL_STATUS_H
> > +
> > +#include <swupdate_status.h>
> > +#include <status_ipc.h>
> > +
> > +/*
> > + * Internal SWUpdate functions to drive the status
> > + * interface. Common status definitions for internal
> > + * as well as external use are defined in status_ipc.h
> > + */
> > +void swupdate_status_info(RECOVERY_STATUS status, int event, int level, const char *msg);
> > +
> > +void *status_thread (void *data);
> > +
> > +#endif
> > diff --git a/include/status_ipc.h b/include/status_ipc.h
> > new file mode 100644
> > index 0000000..fcf70d4
> > --- /dev/null
> > +++ b/include/status_ipc.h
> > @@ -0,0 +1,53 @@
> > +/*
> > + * Author: Christian Storm
> > + * Copyright (C) 2017, Siemens AG
> > + *
> > + * SPDX-License-Identifier:     LGPL-2.1-or-later
> > + */
> > +
> > +#ifndef _STATUS_IPC_H
> > +#define _STATUS_IPC_H
> > +
> > +#include <stdbool.h>
> > +#include <swupdate_status.h>
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +#define PRDESCSIZE   2048
> > +
> > +extern char* SOCKET_STATUS_PATH;
> > +
> > +/*
> > + * Message sent via status socket.
> > + * Data is sent in LE if required.
> > + */
> > +struct status_msg {
> > +     unsigned int    magic;          /* Magic Number */
> > +     int current;
> > +     int level;
> > +     int error;
> > +     unsigned int    desclen;        /* Len of data valid in desc */
> > +     char            desc[PRDESCSIZE]; /* additional information about install */
> > +};
> > +
> > +char *get_status_socket(void);
> > +
> > +/* Standard function to connect to status interface */
> > +int status_ipc_connect(bool reconnect);
> > +
> > +/*
> > + * In case more as an instance of SWUpdate is running, this allows to select
> > + * which should be taken
> > + */
> > +int status_ipc_connect_with_path(const char *socketpath, bool reconnect);
> > +
> > +/* Retrieve messages from status interface (it blocks) */
> > +int status_ipc_receive(int *connfd, struct status_msg *msg);
> > +
> > +#ifdef __cplusplus
> > +}   // extern "C"
> > +#endif
> > +
> > +#endif
> > diff --git a/include/util.h b/include/util.h
> > index 9f29f5f..0086694 100644
> > --- a/include/util.h
> > +++ b/include/util.h
> > @@ -220,6 +220,7 @@ int read_lines_notify(int fd, char *buf, int buf_size, int *buf_offset,
> >                     LOGLEVEL level);
> >   long long get_output_size(struct img_type *img, bool strict);
> >   bool img_check_free_space(struct img_type *img, int fd);
> > +void clean_msg(char *msg, char drop);
> >
> >   /* Decryption key functions */
> >   int load_decryption_key(char *fname);
> > diff --git a/ipc/Makefile b/ipc/Makefile
> > index 71a1f42..661e214 100644
> > --- a/ipc/Makefile
> > +++ b/ipc/Makefile
> > @@ -1,6 +1,6 @@
> >   # Copyright (C) 2014-2018 Stefano Babic <sbabic@denx.de>
> >   #
> >   # SPDX-License-Identifier:     GPL-2.0-only
> > -obj-y                        += network_ipc.o network_ipc-if.o progress_ipc.o
> > +obj-y                        += network_ipc.o network_ipc-if.o progress_ipc.o status_ipc.o
> >
> >   EXTRA_CFLAGS += -fPIC
> > diff --git a/ipc/status_ipc.c b/ipc/status_ipc.c
> > new file mode 100644
> > index 0000000..4d95aee
> > --- /dev/null
> > +++ b/ipc/status_ipc.c
> > @@ -0,0 +1,94 @@
> > +/*
> > + * Author: Christian Storm
> > + * Copyright (C) 2017, Siemens AG
> > + *
> > + * SPDX-License-Identifier:     LGPL-2.1-or-later
> > + */
> > +
> > +#include <sys/socket.h>
> > +#include <sys/un.h>
> > +#include <errno.h>
> > +#include <string.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +#include <stdbool.h>
> > +
> > +#include <status_ipc.h>
> > +
> > +#ifdef CONFIG_SOCKET_STATUS_PATH
> > +char *SOCKET_STATUS_PATH = (char*)CONFIG_SOCKET_STATUS_PATH;
> > +#else
> > +char *SOCKET_STATUS_PATH = NULL;
> > +#endif
> > +
> > +#define SOCKET_STATUS_DEFAULT  "swupdatestatus"
> > +
> > +char *get_status_socket(void) {
> > +     if (!SOCKET_STATUS_PATH || !strlen(SOCKET_STATUS_PATH)) {
> > +             const char *tmpdir = getenv("TMPDIR");
> > +             if (!tmpdir)
> > +                     tmpdir = "/tmp";
> > +
> > +             if (asprintf(&SOCKET_STATUS_PATH, "%s/%s", tmpdir, SOCKET_STATUS_DEFAULT) == -1)
> > +                     return (char *)"/tmp/"SOCKET_STATUS_DEFAULT;
> > +     }
> > +
> > +     return SOCKET_STATUS_PATH;
> > +}
> > +
> > +static int _status_ipc_connect(const char *socketpath, bool reconnect)
> > +{
> > +     struct sockaddr_un servaddr;
> > +     int fd = socket(AF_LOCAL, SOCK_STREAM, 0);
> > +     bzero(&servaddr, sizeof(servaddr));
> > +     servaddr.sun_family = AF_LOCAL;
> > +     strncpy(servaddr.sun_path, socketpath, sizeof(servaddr.sun_path) - 1);
> > +
> > +     /*
> > +      * Check to get a valid socket
> > +      */
> > +     if (fd < 0)
> > +             return -1;
> > +
> > +     do {
> > +             if (connect(fd, (struct sockaddr *) &servaddr, sizeof(servaddr)) == 0) {
> > +                     break;
> > +             }
> > +             if (!reconnect) {
> > +                     fprintf(stderr, "cannot communicate with SWUpdate via %s\n", socketpath);
> > +                     close(fd);
> > +                     return -1;
> > +             }
> > +
> > +             usleep(10000);
> > +     } while (true);
> > +
> > +     fprintf(stdout, "Connected to SWUpdate via %s\n", socketpath);
> > +     return fd;
> > +}
> > +
> > +int status_ipc_connect_with_path(const char *socketpath, bool reconnect) {
> > +     return _status_ipc_connect(socketpath, reconnect);
> > +}
> > +
> > +int status_ipc_connect(bool reconnect)
> > +{
> > +     return _status_ipc_connect(get_status_socket(), reconnect);
> > +}
> > +
> > +int status_ipc_receive(int *connfd, struct status_msg *msg) {
> > +     int ret = read(*connfd, msg, sizeof(*msg));
> > +
> > +     if (ret == -1 && (errno == EAGAIN || errno == EINTR))
> > +             return 0;
> > +
> > +     if (ret != sizeof(*msg)) {
> > +             fprintf(stdout, "Connection closing..\n");
> > +             close(*connfd);
> > +             *connfd = -1;
> > +             return -1;
> > +     }
> > +
> > +     return ret;
> > +}
> > diff --git a/mongoose/mongoose_interface.c b/mongoose/mongoose_interface.c
> > index 2e9416b..350b06e 100644
> > --- a/mongoose/mongoose_interface.c
> > +++ b/mongoose/mongoose_interface.c
> > @@ -25,6 +25,7 @@
> >   #include <mongoose_interface.h>
> >   #include <parselib.h>
> >   #include <progress_ipc.h>
> > +#include <status_ipc.h>
> >   #include <swupdate_settings.h>
> >   #include <time.h>
> >
> > @@ -141,31 +142,44 @@ static void broadcast(struct mg_mgr *mgr, char *str)
> >
> >   static void *broadcast_message_thread(void *data)
> >   {
> > +     int fd = -1;
> > +
> >       for (;;) {
> > -             ipc_message msg;
> > -             int ret = ipc_get_status(&msg);
> > +             struct status_msg msg;
> > +             int ret;
> >
> > -             if (!ret && strlen(msg.data.status.desc) != 0) {
> > +             if (fd < 0)
> > +                     fd = status_ipc_connect(true);
> > +             /*
> > +              * if still fails, try later
> > +              */
> > +             if (fd < 0) {
> > +                     sleep(1);
> > +                     continue;
> > +             }
> > +
> > +             ret = status_ipc_receive(&fd, &msg);
> > +             if (ret != sizeof(msg))
> > +                     return NULL;
> > +
> > +             if (msg.desclen != 0) {
> >                       struct mg_mgr *mgr = (struct mg_mgr *) data;
> >                       char text[4096];
> >                       char str[4160];
> >
> > -                     snescape(text, sizeof(text), msg.data.status.desc);
> > +                     snescape(text, sizeof(text), msg.desc);
> >
> >                       snprintf(str, sizeof(str),
> > -                             "{\r\n"
> > -                             "\t\"type\": \"message\",\r\n"
> > -                             "\t\"level\": \"%d\",\r\n"
> > -                             "\t\"text\": \"%s\"\r\n"
> > -                             "}\r\n",
> > -                             (msg.data.status.error) ? 3 : 6, /* RFC 5424 */
> > -                             text);
> > +                                      "{\r\n"
> > +                                      "\t\"type\": \"message\",\r\n"
> > +                                      "\t\"level\": \"%d\",\r\n"
> > +                                      "\t\"text\": \"%s\"\r\n"
> > +                                      "}\r\n",
> > +                                      msg.level, /* RFC 5424 */
> > +                                      text);
> >
> >                       broadcast(mgr, str);
> > -                     continue;
> >               }
> > -
> > -             usleep(50 * 1000);
> >       }
> >
> >       return NULL;
> >
>
> --
> =====================================================================
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de
> =====================================================================

Hi James,

On 07.09.21 15:42, James Hilliard wrote:
> On Sun, Aug 15, 2021 at 8:35 AM Stefano Babic <sbabic@denx.de> wrote:
>>
>> Hi James,
>>
>> On 15.08.21 05:46, James Hilliard wrote:
>>> Currently the only way to get status messages is by calling the
>>> ctrl IPC interface, however this has the disadvantage of only
>>> effectively allowing a single client at a time to read status
>>> messages as they are deleted when read using this interface.
>>
>> This is true.
>>
>>>
>>> In order to allow multiple clients to read status messages
>>> add a dedicated interface for those messages similar to the
>>> progress interface.
>>>
>>
>> However, what I see here is that the code is copied and duplicated. I am
>> also concerning about creating an additional socket interface, when
>> there is already two sockets, one to control SWUpdate (and send a SWU),
>> one to get information about the update. The additional status socker is
>> mostly a copy frim the progress and it is also unidirectional, that mean
>> clients just want to get informed and they do not send data back. More
>> or less, the limits of GET_STATUS led me to create the progress
>> interface. The GET_STATUS works also in polling mode, while the progress
>> interface sends events to the listen. For external interface, a syslog
>> notifier sends the same data to the syslog socket.
>>
>> If this is really required, I do not like to get duplicated code. It
>> should be solved reusing in some way the progress interface (and taking
>> care of some compatibility issues that could be raised).
> 
> Extending the progress interface seemed somewhat problematic for backwards
> compatibility, at least I didn't see a good way to handle that,
> however I think I
> managed to get it working reasonably well by extending the main network thread
> here:
> https://groups.google.com/g/swupdate/c/eqzLb4YKY7A
> 
> Does this approach look ok to you? One reason I went with this approach is that
> the msgdata response structure format is reusable for these
> notifications as it is
> already the right size.

It looks ok for me, but I would like to test it myself with some 
projects of mine before merging, but I have not yet found enough time. 
It is on my queue.

Regards,
Stefano

> 
> I also wanted to make sure that clients wouldn't lose messages if they connected
> slightly late so I made it so that this interface does not purge
> entries from the
> notifymsgs buffer unless GET_STATUS is called(I kept existing behavior with this
> for backwards compatibility).
> 
>>
>> Best regards,
>> Stefano
>>
>>> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
>>> ---
>>>    Kconfig                       |   5 +
>>>    core/Makefile                 |   1 +
>>>    core/network_thread.c         |  10 --
>>>    core/notifier.c               |  14 +++
>>>    core/status_thread.c          | 176 ++++++++++++++++++++++++++++++++++
>>>    core/swupdate.c               |   3 +
>>>    core/util.c                   |  10 ++
>>>    include/status.h              |  23 +++++
>>>    include/status_ipc.h          |  53 ++++++++++
>>>    include/util.h                |   1 +
>>>    ipc/Makefile                  |   2 +-
>>>    ipc/status_ipc.c              |  94 ++++++++++++++++++
>>>    mongoose/mongoose_interface.c |  42 +++++---
>>>    13 files changed, 409 insertions(+), 25 deletions(-)
>>>    create mode 100644 core/status_thread.c
>>>    create mode 100644 include/status.h
>>>    create mode 100644 include/status_ipc.h
>>>    create mode 100644 ipc/status_ipc.c
>>>
>>> diff --git a/Kconfig b/Kconfig
>>> index dc86957..7fc7382 100644
>>> --- a/Kconfig
>>> +++ b/Kconfig
>>> @@ -215,6 +215,11 @@ config SOCKET_PROGRESS_PATH
>>>        help
>>>          Path to the socket progress information is sent to.
>>>
>>> +config SOCKET_STATUS_PATH
>>> +     string "SWUpdate status socket path"
>>> +     help
>>> +       Path to the socket status information is sent to.
>>> +
>>>    config SOCKET_NOTIFIER_DIRECTORY
>>>        string "SWUpdate notifier socket directory"
>>>        depends on HAVE_FREEBSD
>>> diff --git a/core/Makefile b/core/Makefile
>>> index fa30e6e..e6aeba4 100644
>>> --- a/core/Makefile
>>> +++ b/core/Makefile
>>> @@ -22,6 +22,7 @@ obj-y += swupdate.o \
>>>         network_thread.o \
>>>         stream_interface.o \
>>>         progress_thread.o \
>>> +      status_thread.o \
>>>         parsing_library.o \
>>>         artifacts_versions.o \
>>>         swupdate_dict.o \
>>> diff --git a/core/network_thread.c b/core/network_thread.c
>>> index adaf21c..59f1c15 100644
>>> --- a/core/network_thread.c
>>> +++ b/core/network_thread.c
>>> @@ -106,16 +106,6 @@ static bool is_selection_allowed(const char *software_set, char *running_mode,
>>>        return allowed;
>>>    }
>>>
>>> -static void clean_msg(char *msg, char drop)
>>> -{
>>> -     char *lfpos;
>>> -     lfpos = strchr(msg, drop);
>>> -     while (lfpos) {
>>> -             *lfpos = ' ';
>>> -             lfpos = strchr(msg, drop);
>>> -     }
>>> -}
>>> -
>>>    static void network_notifier(RECOVERY_STATUS status, int error, int level, const char *msg)
>>>    {
>>>        int len = msg ? strlen(msg) : 0;
>>> diff --git a/core/notifier.c b/core/notifier.c
>>> index 810769c..7510f93 100644
>>> --- a/core/notifier.c
>>> +++ b/core/notifier.c
>>> @@ -21,6 +21,7 @@
>>>    #include "util.h"
>>>    #include "pctl.h"
>>>    #include "progress.h"
>>> +#include "status.h"
>>>
>>>    #ifdef CONFIG_SYSTEMD
>>>    #include <sys/stat.h>
>>> @@ -357,6 +358,18 @@ static void progress_notifier (RECOVERY_STATUS status, int event, int level, con
>>>        swupdate_progress_info(status, event, msg);
>>>    }
>>>
>>> +/*
>>> + * Status notifier: the message should be forwarded to the status
>>> + * interface only.
>>> + */
>>> +static void status_notifier (RECOVERY_STATUS status, int event, int level, const char *msg)
>>> +{
>>> +     if (status == PROGRESS || event == RECOVERY_DWL)
>>> +             return;
>>> +
>>> +     swupdate_status_info(status, event, level, msg);
>>> +}
>>> +
>>>
>>>    #if defined(__FreeBSD__)
>>>    static char* socket_path = NULL;
>>> @@ -526,6 +539,7 @@ void notify_init(void)
>>>                register_notifier(console_notifier);
>>>                register_notifier(process_notifier);
>>>                register_notifier(progress_notifier);
>>> +             register_notifier(status_notifier);
>>>                start_thread(notifier_thread, NULL);
>>>        }
>>>    }
>>> diff --git a/core/status_thread.c b/core/status_thread.c
>>> new file mode 100644
>>> index 0000000..e950144
>>> --- /dev/null
>>> +++ b/core/status_thread.c
>>> @@ -0,0 +1,176 @@
>>> +/*
>>> + * (C) Copyright 2016
>>> + * Stefano Babic, DENX Software Engineering, sbabic@denx.de.
>>> + *
>>> + * SPDX-License-Identifier:     GPL-2.0-only
>>> + */
>>> +
>>> +#include <stdio.h>
>>> +#include <stdlib.h>
>>> +#include <stdbool.h>
>>> +#include <unistd.h>
>>> +#include <string.h>
>>> +#include <errno.h>
>>> +#include <sys/ioctl.h>
>>> +#include <fcntl.h>
>>> +#include <sys/types.h>
>>> +#include <sys/socket.h>
>>> +#include <sys/stat.h>
>>> +#include <sys/un.h>
>>> +#include <sys/select.h>
>>> +#include <arpa/inet.h>
>>> +#include <netinet/in.h>
>>> +#include <pthread.h>
>>> +
>>> +#include "swupdate.h"
>>> +#include <handler.h>
>>> +#include "util.h"
>>> +#include "pctl.h"
>>> +#include "network_ipc.h"
>>> +#include "network_interface.h"
>>> +#include <status.h>
>>> +#include "generated/autoconf.h"
>>> +
>>> +#ifdef CONFIG_SYSTEMD
>>> +#include <systemd/sd-daemon.h>
>>> +#endif
>>> +
>>> +struct status_conn {
>>> +     SIMPLEQ_ENTRY(status_conn) next;
>>> +     int sockfd;
>>> +};
>>> +
>>> +SIMPLEQ_HEAD(connections, status_conn);
>>> +
>>> +/*
>>> + * Structure contains data regarding
>>> + * current installation
>>> + */
>>> +struct swupdate_status {
>>> +     struct status_msg msg;
>>> +     char *current_image;
>>> +     const handler *curhnd;
>>> +     struct connections conns;
>>> +     pthread_mutex_t lock;
>>> +     bool step_running;
>>> +};
>>> +static struct swupdate_status notification;
>>> +
>>> +/*
>>> + * This must be called after acquiring the mutex
>>> + * for the status structure
>>> + */
>>> +static void send_status_msg(void)
>>> +{
>>> +     struct status_conn *conn, *tmp;
>>> +     struct swupdate_status *nmsg = &notification;
>>> +     void *buf;
>>> +     size_t count;
>>> +     ssize_t n;
>>> +
>>> +     SIMPLEQ_FOREACH_SAFE(conn, &nmsg->conns, next, tmp) {
>>> +             buf = &nmsg->msg;
>>> +             count = sizeof(nmsg->msg);
>>> +             while (count > 0) {
>>> +                     n = send(conn->sockfd, buf, count, MSG_NOSIGNAL);
>>> +                     if (n <= 0) {
>>> +                             if (n == 0) {
>>> +                                     TRACE("A status client is not responding, removing it.");
>>> +                             } else {
>>> +                                     TRACE("A status client disappeared, removing it: %s", strerror(errno));
>>> +                             }
>>> +                             close(conn->sockfd);
>>> +                             SIMPLEQ_REMOVE(&nmsg->conns, conn,
>>> +                                             status_conn, next);
>>> +                             free(conn);
>>> +                             break;
>>> +                     }
>>> +                     count -= (size_t)n;
>>> +                     buf = (char*)buf + n;
>>> +             }
>>> +     }
>>> +}
>>> +
>>> +void swupdate_status_info(RECOVERY_STATUS status, int error, int level, const char *msg)
>>> +{
>>> +     struct swupdate_status *nmsg = &notification;
>>> +     pthread_mutex_lock(&nmsg->lock);
>>> +     memset(&nmsg->msg, 0, sizeof(nmsg->msg));
>>> +     if (msg) {
>>> +             strncpy(nmsg->msg.desc, msg,
>>> +                             sizeof(nmsg->msg.desc) - 1);
>>> +             clean_msg(nmsg->msg.desc, '\t');
>>> +             clean_msg(nmsg->msg.desc, '\n');
>>> +             clean_msg(nmsg->msg.desc, '\r');
>>> +     }
>>> +     nmsg->msg.current = status;
>>> +     nmsg->msg.level = level;
>>> +     nmsg->msg.error = error;
>>> +     nmsg->msg.desclen = strlen(nmsg->msg.desc);
>>> +     send_status_msg();
>>> +     pthread_mutex_unlock(&nmsg->lock);
>>> +}
>>> +
>>> +static void unlink_socket(void)
>>> +{
>>> +#ifdef CONFIG_SYSTEMD
>>> +     if (sd_booted() && sd_listen_fds(0) > 0) {
>>> +             /*
>>> +              * There were socket fds handed-over by systemd,
>>> +              * so don't delete the socket file.
>>> +              */
>>> +             return;
>>> +     }
>>> +#endif
>>> +     unlink(get_status_socket());
>>> +}
>>> +
>>> +void *status_thread (void __attribute__ ((__unused__)) *data)
>>> +{
>>> +     int listen, connfd;
>>> +     socklen_t clilen;
>>> +     struct sockaddr_un cliaddr;
>>> +     struct swupdate_status *nmsg = &notification;
>>> +     struct status_conn *conn;
>>> +
>>> +     pthread_mutex_init(&nmsg->lock, NULL);
>>> +     SIMPLEQ_INIT(&nmsg->conns);
>>> +
>>> +     /* Initialize and bind to UDS */
>>> +     listen = listener_create(get_status_socket(), SOCK_STREAM);
>>> +     if (listen < 0 ) {
>>> +             ERROR("Error creating IPC socket %s, exiting.", get_status_socket());
>>> +             exit(2);
>>> +     }
>>> +
>>> +     if (atexit(unlink_socket) != 0) {
>>> +             TRACE("Cannot setup socket cleanup on exit, %s won't be unlinked.",
>>> +                     get_status_socket());
>>> +     }
>>> +
>>> +     thread_ready();
>>> +     do {
>>> +             clilen = sizeof(cliaddr);
>>> +             if ( (connfd = accept(listen, (struct sockaddr *) &cliaddr, &clilen)) < 0) {
>>> +                     if (errno == EINTR)
>>> +                             continue;
>>> +                     else {
>>> +                             TRACE("Accept returns: %s", strerror(errno));
>>> +                             continue;
>>> +                     }
>>> +             }
>>> +
>>> +             /*
>>> +              * Save the new connection to be handled by the status thread
>>> +              */
>>> +             conn = (struct status_conn *)calloc(1, sizeof(*conn));
>>> +             if (!conn) {
>>> +                     ERROR("Out of memory, skipping...");
>>> +                     continue;
>>> +             }
>>> +             conn->sockfd = connfd;
>>> +             pthread_mutex_lock(&nmsg->lock);
>>> +             SIMPLEQ_INSERT_TAIL(&nmsg->conns, conn, next);
>>> +             pthread_mutex_unlock(&nmsg->lock);
>>> +     } while(1);
>>> +}
>>> diff --git a/core/swupdate.c b/core/swupdate.c
>>> index 949a647..fe0b92b 100644
>>> --- a/core/swupdate.c
>>> +++ b/core/swupdate.c
>>> @@ -43,6 +43,7 @@
>>>    #include "sslapi.h"
>>>    #include "suricatta/suricatta.h"
>>>    #include "progress.h"
>>> +#include "status.h"
>>>    #include "parselib.h"
>>>    #include "swupdate_settings.h"
>>>    #include "pctl.h"
>>> @@ -860,6 +861,8 @@ int main(int argc, char **argv)
>>>
>>>        start_thread(progress_bar_thread, NULL);
>>>
>>> +     start_thread(status_thread, NULL);
>>> +
>>>        /* wait for threads to be done before starting children */
>>>        wait_threads_ready();
>>>
>>> diff --git a/core/util.c b/core/util.c
>>> index 6188650..e54d156 100644
>>> --- a/core/util.c
>>> +++ b/core/util.c
>>> @@ -1137,3 +1137,13 @@ bool img_check_free_space(struct img_type *img, int fd)
>>>
>>>        return check_free_space(fd, size, img->fname);
>>>    }
>>> +
>>> +void clean_msg(char *msg, char drop)
>>> +{
>>> +     char *lfpos;
>>> +     lfpos = strchr(msg, drop);
>>> +     while (lfpos) {
>>> +             *lfpos = ' ';
>>> +             lfpos = strchr(msg, drop);
>>> +     }
>>> +}
>>> diff --git a/include/status.h b/include/status.h
>>> new file mode 100644
>>> index 0000000..a22a8f5
>>> --- /dev/null
>>> +++ b/include/status.h
>>> @@ -0,0 +1,23 @@
>>> +/*
>>> + * (C) Copyright 2016
>>> + * Stefano Babic, DENX Software Engineering, sbabic@denx.de.
>>> + *
>>> + * SPDX-License-Identifier:     GPL-2.0-only
>>> + */
>>> +
>>> +#ifndef _INSTALL_STATUS_H
>>> +#define _INSTALL_STATUS_H
>>> +
>>> +#include <swupdate_status.h>
>>> +#include <status_ipc.h>
>>> +
>>> +/*
>>> + * Internal SWUpdate functions to drive the status
>>> + * interface. Common status definitions for internal
>>> + * as well as external use are defined in status_ipc.h
>>> + */
>>> +void swupdate_status_info(RECOVERY_STATUS status, int event, int level, const char *msg);
>>> +
>>> +void *status_thread (void *data);
>>> +
>>> +#endif
>>> diff --git a/include/status_ipc.h b/include/status_ipc.h
>>> new file mode 100644
>>> index 0000000..fcf70d4
>>> --- /dev/null
>>> +++ b/include/status_ipc.h
>>> @@ -0,0 +1,53 @@
>>> +/*
>>> + * Author: Christian Storm
>>> + * Copyright (C) 2017, Siemens AG
>>> + *
>>> + * SPDX-License-Identifier:     LGPL-2.1-or-later
>>> + */
>>> +
>>> +#ifndef _STATUS_IPC_H
>>> +#define _STATUS_IPC_H
>>> +
>>> +#include <stdbool.h>
>>> +#include <swupdate_status.h>
>>> +
>>> +#ifdef __cplusplus
>>> +extern "C" {
>>> +#endif
>>> +
>>> +#define PRDESCSIZE   2048
>>> +
>>> +extern char* SOCKET_STATUS_PATH;
>>> +
>>> +/*
>>> + * Message sent via status socket.
>>> + * Data is sent in LE if required.
>>> + */
>>> +struct status_msg {
>>> +     unsigned int    magic;          /* Magic Number */
>>> +     int current;
>>> +     int level;
>>> +     int error;
>>> +     unsigned int    desclen;        /* Len of data valid in desc */
>>> +     char            desc[PRDESCSIZE]; /* additional information about install */
>>> +};
>>> +
>>> +char *get_status_socket(void);
>>> +
>>> +/* Standard function to connect to status interface */
>>> +int status_ipc_connect(bool reconnect);
>>> +
>>> +/*
>>> + * In case more as an instance of SWUpdate is running, this allows to select
>>> + * which should be taken
>>> + */
>>> +int status_ipc_connect_with_path(const char *socketpath, bool reconnect);
>>> +
>>> +/* Retrieve messages from status interface (it blocks) */
>>> +int status_ipc_receive(int *connfd, struct status_msg *msg);
>>> +
>>> +#ifdef __cplusplus
>>> +}   // extern "C"
>>> +#endif
>>> +
>>> +#endif
>>> diff --git a/include/util.h b/include/util.h
>>> index 9f29f5f..0086694 100644
>>> --- a/include/util.h
>>> +++ b/include/util.h
>>> @@ -220,6 +220,7 @@ int read_lines_notify(int fd, char *buf, int buf_size, int *buf_offset,
>>>                      LOGLEVEL level);
>>>    long long get_output_size(struct img_type *img, bool strict);
>>>    bool img_check_free_space(struct img_type *img, int fd);
>>> +void clean_msg(char *msg, char drop);
>>>
>>>    /* Decryption key functions */
>>>    int load_decryption_key(char *fname);
>>> diff --git a/ipc/Makefile b/ipc/Makefile
>>> index 71a1f42..661e214 100644
>>> --- a/ipc/Makefile
>>> +++ b/ipc/Makefile
>>> @@ -1,6 +1,6 @@
>>>    # Copyright (C) 2014-2018 Stefano Babic <sbabic@denx.de>
>>>    #
>>>    # SPDX-License-Identifier:     GPL-2.0-only
>>> -obj-y                        += network_ipc.o network_ipc-if.o progress_ipc.o
>>> +obj-y                        += network_ipc.o network_ipc-if.o progress_ipc.o status_ipc.o
>>>
>>>    EXTRA_CFLAGS += -fPIC
>>> diff --git a/ipc/status_ipc.c b/ipc/status_ipc.c
>>> new file mode 100644
>>> index 0000000..4d95aee
>>> --- /dev/null
>>> +++ b/ipc/status_ipc.c
>>> @@ -0,0 +1,94 @@
>>> +/*
>>> + * Author: Christian Storm
>>> + * Copyright (C) 2017, Siemens AG
>>> + *
>>> + * SPDX-License-Identifier:     LGPL-2.1-or-later
>>> + */
>>> +
>>> +#include <sys/socket.h>
>>> +#include <sys/un.h>
>>> +#include <errno.h>
>>> +#include <string.h>
>>> +#include <stdio.h>
>>> +#include <stdlib.h>
>>> +#include <unistd.h>
>>> +#include <stdbool.h>
>>> +
>>> +#include <status_ipc.h>
>>> +
>>> +#ifdef CONFIG_SOCKET_STATUS_PATH
>>> +char *SOCKET_STATUS_PATH = (char*)CONFIG_SOCKET_STATUS_PATH;
>>> +#else
>>> +char *SOCKET_STATUS_PATH = NULL;
>>> +#endif
>>> +
>>> +#define SOCKET_STATUS_DEFAULT  "swupdatestatus"
>>> +
>>> +char *get_status_socket(void) {
>>> +     if (!SOCKET_STATUS_PATH || !strlen(SOCKET_STATUS_PATH)) {
>>> +             const char *tmpdir = getenv("TMPDIR");
>>> +             if (!tmpdir)
>>> +                     tmpdir = "/tmp";
>>> +
>>> +             if (asprintf(&SOCKET_STATUS_PATH, "%s/%s", tmpdir, SOCKET_STATUS_DEFAULT) == -1)
>>> +                     return (char *)"/tmp/"SOCKET_STATUS_DEFAULT;
>>> +     }
>>> +
>>> +     return SOCKET_STATUS_PATH;
>>> +}
>>> +
>>> +static int _status_ipc_connect(const char *socketpath, bool reconnect)
>>> +{
>>> +     struct sockaddr_un servaddr;
>>> +     int fd = socket(AF_LOCAL, SOCK_STREAM, 0);
>>> +     bzero(&servaddr, sizeof(servaddr));
>>> +     servaddr.sun_family = AF_LOCAL;
>>> +     strncpy(servaddr.sun_path, socketpath, sizeof(servaddr.sun_path) - 1);
>>> +
>>> +     /*
>>> +      * Check to get a valid socket
>>> +      */
>>> +     if (fd < 0)
>>> +             return -1;
>>> +
>>> +     do {
>>> +             if (connect(fd, (struct sockaddr *) &servaddr, sizeof(servaddr)) == 0) {
>>> +                     break;
>>> +             }
>>> +             if (!reconnect) {
>>> +                     fprintf(stderr, "cannot communicate with SWUpdate via %s\n", socketpath);
>>> +                     close(fd);
>>> +                     return -1;
>>> +             }
>>> +
>>> +             usleep(10000);
>>> +     } while (true);
>>> +
>>> +     fprintf(stdout, "Connected to SWUpdate via %s\n", socketpath);
>>> +     return fd;
>>> +}
>>> +
>>> +int status_ipc_connect_with_path(const char *socketpath, bool reconnect) {
>>> +     return _status_ipc_connect(socketpath, reconnect);
>>> +}
>>> +
>>> +int status_ipc_connect(bool reconnect)
>>> +{
>>> +     return _status_ipc_connect(get_status_socket(), reconnect);
>>> +}
>>> +
>>> +int status_ipc_receive(int *connfd, struct status_msg *msg) {
>>> +     int ret = read(*connfd, msg, sizeof(*msg));
>>> +
>>> +     if (ret == -1 && (errno == EAGAIN || errno == EINTR))
>>> +             return 0;
>>> +
>>> +     if (ret != sizeof(*msg)) {
>>> +             fprintf(stdout, "Connection closing..\n");
>>> +             close(*connfd);
>>> +             *connfd = -1;
>>> +             return -1;
>>> +     }
>>> +
>>> +     return ret;
>>> +}
>>> diff --git a/mongoose/mongoose_interface.c b/mongoose/mongoose_interface.c
>>> index 2e9416b..350b06e 100644
>>> --- a/mongoose/mongoose_interface.c
>>> +++ b/mongoose/mongoose_interface.c
>>> @@ -25,6 +25,7 @@
>>>    #include <mongoose_interface.h>
>>>    #include <parselib.h>
>>>    #include <progress_ipc.h>
>>> +#include <status_ipc.h>
>>>    #include <swupdate_settings.h>
>>>    #include <time.h>
>>>
>>> @@ -141,31 +142,44 @@ static void broadcast(struct mg_mgr *mgr, char *str)
>>>
>>>    static void *broadcast_message_thread(void *data)
>>>    {
>>> +     int fd = -1;
>>> +
>>>        for (;;) {
>>> -             ipc_message msg;
>>> -             int ret = ipc_get_status(&msg);
>>> +             struct status_msg msg;
>>> +             int ret;
>>>
>>> -             if (!ret && strlen(msg.data.status.desc) != 0) {
>>> +             if (fd < 0)
>>> +                     fd = status_ipc_connect(true);
>>> +             /*
>>> +              * if still fails, try later
>>> +              */
>>> +             if (fd < 0) {
>>> +                     sleep(1);
>>> +                     continue;
>>> +             }
>>> +
>>> +             ret = status_ipc_receive(&fd, &msg);
>>> +             if (ret != sizeof(msg))
>>> +                     return NULL;
>>> +
>>> +             if (msg.desclen != 0) {
>>>                        struct mg_mgr *mgr = (struct mg_mgr *) data;
>>>                        char text[4096];
>>>                        char str[4160];
>>>
>>> -                     snescape(text, sizeof(text), msg.data.status.desc);
>>> +                     snescape(text, sizeof(text), msg.desc);
>>>
>>>                        snprintf(str, sizeof(str),
>>> -                             "{\r\n"
>>> -                             "\t\"type\": \"message\",\r\n"
>>> -                             "\t\"level\": \"%d\",\r\n"
>>> -                             "\t\"text\": \"%s\"\r\n"
>>> -                             "}\r\n",
>>> -                             (msg.data.status.error) ? 3 : 6, /* RFC 5424 */
>>> -                             text);
>>> +                                      "{\r\n"
>>> +                                      "\t\"type\": \"message\",\r\n"
>>> +                                      "\t\"level\": \"%d\",\r\n"
>>> +                                      "\t\"text\": \"%s\"\r\n"
>>> +                                      "}\r\n",
>>> +                                      msg.level, /* RFC 5424 */
>>> +                                      text);
>>>
>>>                        broadcast(mgr, str);
>>> -                     continue;
>>>                }
>>> -
>>> -             usleep(50 * 1000);
>>>        }
>>>
>>>        return NULL;
>>>
>>
>> --
>> =====================================================================
>> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de
>> =====================================================================
>