Re: [PATCH v4 09/10] libmpathcmd: honor MULTIPATH_SOCKET_NAME environment variable

[Benjamin Marzinski <bmarzins@redhat.com>]

On Fri, Feb 21, 2025 at 09:41:08PM +0100, Martin Wilck wrote:
> In systemd installments, users can already override the socket names
> that systemd listens on. With this patch, clients using libmpathcmd
> can be customized to use a non-standard socket by setting an environment
> variable.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmpathcmd/mpath_cmd.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/libmpathcmd/mpath_cmd.c b/libmpathcmd/mpath_cmd.c
> index 83cb6ad..5a23236 100644
> --- a/libmpathcmd/mpath_cmd.c
> +++ b/libmpathcmd/mpath_cmd.c
> @@ -20,6 +20,7 @@
>  #include <stdlib.h>
>  #include <unistd.h>
>  #include <stdio.h>
> +#include <stdlib.h>
>  #include <sys/types.h>
>  #include <sys/socket.h>
>  #include <sys/un.h>
> @@ -104,6 +105,7 @@ int mpath_connect__(int nonblocking)
>  	int flags = 0;
>  	const char *const names[2] = {PATHNAME_SOCKET, ABSTRACT_SOCKET};
>  	int name_idx = 0;
> +	const char *env_name = getenv("MULTIPATH_SOCKET_NAME"), *name;
>  
>  retry:
>  	fd = socket(AF_LOCAL, SOCK_STREAM, 0);
> @@ -116,12 +118,13 @@ retry:
>  			(void)fcntl(fd, F_SETFL, flags|O_NONBLOCK);
>  	}
>  
> -	len = mpath_fill_sockaddr__(&addr, names[name_idx]);
> +	name = env_name ? env_name : names[name_idx];
> +	len = mpath_fill_sockaddr__(&addr, name);
>  	if (connect(fd, (struct sockaddr *)&addr, len) == -1) {
>  		int err = errno;
>  
>  		close(fd);
> -		if (++name_idx == 1)

You lost a "+" from ++name_idx

Otherwise, looks good. Assuming that gets updated when you push it to
your queue branch:

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

> +		if (name != env_name && +name_idx == 1)
>  			goto retry;
>  		else {
>  			errno = err;
> -- 
> 2.48.1