[systemd-devel] [PATCH 04/12] Add some plymouth functionality to connect and send, messages

[Zbigniew Jędrzejewski-Szmek]

On Wed, Jan 28, 2015 at 02:22:04PM +0100, Didier Roche wrote:
> 

> From 7afe9270e3210668053089caaff8a1dd790a48f5 Mon Sep 17 00:00:00 2001
> From: Didier Roche <didrocks at ubuntu.com>
> Date: Mon, 26 Jan 2015 17:07:32 +0100
> Subject: [PATCH 04/12] Add some plymouth functionality to connect and send
>  messages
> 
> Connect to plymouth (if running).
> Automatic reconnect if plymouth was disconnected (or respawned)
> when trying to send a subsequent message.
> ---
>  Makefile.am           |  2 ++
>  configure.ac          | 12 ++++++++
>  src/shared/plymouth.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/shared/plymouth.h |  8 +++++
>  4 files changed, 105 insertions(+)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 18be607..a9d61f1 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -929,10 +929,12 @@ libsystemd_shared_la_CFLAGS = \
>  	$(AM_CFLAGS) \
>  	$(CAP_CFLAGS) \
>  	$(SECCOMP_CFLAGS) \
> +	$(PLYMOUTH_CFLAGS) \
>  	-pthread
>  
>  libsystemd_shared_la_LIBADD = \
>  	$(CAP_LIBS) \
> +	$(PLYMOUTH_LIBS) \
>  	-lm
>  
>  # ------------------------------------------------------------------------------
> diff --git a/configure.ac b/configure.ac
> index 12e4ab2..62f1eef 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -857,6 +857,18 @@ fi
>  AM_CONDITIONAL(HAVE_MICROHTTPD, [test "$have_microhttpd" = "yes"])
>  
>  # ------------------------------------------------------------------------------
> +have_plymouth=no
> +AC_ARG_ENABLE(plymouth, AS_HELP_STRING([--disable-plymouth], [disable plymouth integration]))
> +if test "x$enable_plymouth" != "xno"; then
> +        PKG_CHECK_MODULES([PLYMOUTH], [ply-boot-client >= 0.8.0],
> +                [AC_DEFINE(HAVE_PLYMOUTH, 1, [Define if plymouth is available]) have_plymouth=yes], have_plymouth=no)
> +        if test "x$have_plymouth" = xno -a "x$enable_plymouth" = xyes; then
> +                AC_MSG_ERROR([*** plymouth integration requested but libraries not found])
> +        fi
> +fi
> +AM_CONDITIONAL(HAVE_PLYMOUTH, [test "$have_plymouth" = "yes"])
> +
> +# ------------------------------------------------------------------------------
>  have_gnutls=no
>  AC_ARG_ENABLE(gnutls, AS_HELP_STRING([--disable-gnutls], [disable gnutls support]))
>  if test "x$enable_gnutls" != "xno"; then
> diff --git a/src/shared/plymouth.c b/src/shared/plymouth.c
> index c4865dd..f7155c4 100644
> --- a/src/shared/plymouth.c
> +++ b/src/shared/plymouth.c
> @@ -19,13 +19,96 @@
>    along with systemd; If not, see <http://www.gnu.org/licenses/>.
>  ***/
>  
> +#ifdef HAVE_PLYMOUTH
> +#include <ply-boot-client.h>
> +#endif
> +
>  #include <stdbool.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <unistd.h>
>  
> +#include "log.h"
>  #include "plymouth.h"
>  
> +#ifdef HAVE_PLYMOUTH
> +void plymouth_disconnected (void *user_data, ply_boot_client_t *client);
> +void plymouth_update_failed(void *user_data, ply_boot_client_t *client);
> +
> +static ply_boot_client_t *plymouth_client = NULL;
> +static ply_event_loop_t *plymouth_event_loop = NULL;
> +#endif
> +
>  bool plymouth_running(void) {
>          return access("/run/plymouth/pid", F_OK) >= 0;
>  }
> +
> +#ifdef HAVE_PLYMOUTH
> +bool plymouth_connect(void) {
Is there a particular reason why this cannot return a normal int code?

> +
> +        /* Keep existing connection */
> +        if (plymouth_client)
> +                return true;
> +
> +        /* Create the event loop */
> +        if (!plymouth_event_loop)
Please make this {

> +                plymouth_event_loop = ply_event_loop_new();
> +
> +        if (!plymouth_event_loop) {
> +                log_error("Couldn't create event loop for plymouth");
> +                return false;
> +        }

} a subclause of the if. It looks weird otherwise.

> +
> +        plymouth_client = ply_boot_client_new();
> +
> +        if (!ply_boot_client_connect(plymouth_client, plymouth_disconnected, NULL)) {
> +                log_error("Couldn't connect to plymouth");
> +                ply_boot_client_free(plymouth_client);
> +                plymouth_client = NULL;
> +                plymouth_event_loop = NULL;
> +                return false;
> +        }
> +
> +        /* attach event loop after being connected to plymouth or the disconnect handler won't be registered
> +           and flush all events that may exists from an older connection if we are reconnected */
> +        ply_boot_client_attach_to_event_loop(plymouth_client, plymouth_event_loop);
> +        ply_boot_client_flush(plymouth_client);
> +
> +        return true;
> +}
> +
> +void plymouth_disconnect(void) {
> +        if (!plymouth_client)
> +                 return;
> +        ply_boot_client_disconnect(plymouth_client);
> +        ply_boot_client_flush(plymouth_client);
> +}
> +
> +void plymouth_update(const char *message) {
> +        if (!plymouth_running() || !plymouth_connect())
> +                return;
> +
> +        ply_boot_client_update_daemon(plymouth_client, message, NULL, NULL, NULL);
> +        ply_boot_client_flush(plymouth_client);
> +}
> +
> +void plymouth_delete_message(void) {
> +        if (!plymouth_running() || !plymouth_client)
> +                return;
> +
> +        ply_boot_client_tell_daemon_to_display_message (plymouth_client, "", NULL, plymouth_update_failed, NULL);
> +        ply_boot_client_flush(plymouth_client);
> +}
> +
> +void plymouth_update_failed(void *user_data, ply_boot_client_t *client) {
> +        log_error("Couldn't send message to plymouth");
> +}
> +
> +void plymouth_disconnected (void *user_data, ply_boot_client_t *client) {
Those functions which are not defined in plymouth.h should be static.

> +        log_warning("Plymouth disconnected");
> +        plymouth_delete_message();
> +        ply_boot_client_disconnect(plymouth_client);
> +        ply_boot_client_free(plymouth_client);
> +        plymouth_client = NULL;
> +}
> +#endif
> diff --git a/src/shared/plymouth.h b/src/shared/plymouth.h
> index 39c8c37..15cecb7 100644
> --- a/src/shared/plymouth.h
> +++ b/src/shared/plymouth.h
> @@ -24,3 +24,11 @@
>  #include <stdbool.h>
>  
>  bool plymouth_running(void);
> +
> +#ifdef HAVE_PLYMOUTH
> +bool plymouth_connect(void);
> +void plymouth_disconnect(void);
> +void plymouth_update(const char *message);
> +
> +void plymouth_delete_message(void);
> +#endif

Zbyszek