Re: [Libguestfs] [libnbd PATCH v3 12/29] socket activation: clean up responsibilities of prep.sock.act.env.()

On Wed, Feb 15, 2023 at 03:11:41PM +0100, Laszlo Ersek wrote: > prepare_socket_activation_environment() is a construction function that is > supposed to fill in a string_vector object from the ground up. Right now > it has its responsibilities mixed up in two ways: > > - it expects the caller to pass in a previously re-set string_vector, > > - if it fails, it calls set_error() internally (with a blanket reference > to "malloc"). > > Fix both warts: > > - pass in an *uninitialized* (only allocated) string vector from the > caller, and initialize it in prepare_socket_activation_environment(), > > - move the set_error() call out to the caller. > > Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; > --- > generator/states-connect-socket-activation.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/generator/states-connect-socket-activation.c b/generator/states-connect-socket-activation.c > index c46a0bf5c0a3..b5e146539cc8 100644 > --- a/generator/states-connect-socket-activation.c > +++ b/generator/states-connect-socket-activation.c > @@ -51,7 +51,7 @@ prepare_socket_activation_environment (string_vector *env) > char *p; > size_t i; > > - assert (env->len == 0); > + *env = (string_vector)empty_vector; Do you actually need to cast this?

> /* Reserve slots env[0] and env[1]. */ > p = strdup ("LISTEN_PID=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"); > @@ -90,7 +90,6 @@ prepare_socket_activation_environment (string_vector *env) > return 0; > > err: > - set_error (errno, "malloc");

> string_vector_empty (env); > return -1; > } > @@ -99,7 +98,7 @@ STATE_MACHINE { > CONNECT_SA.START: > int s; > struct sockaddr_un addr; > - string_vector env = empty_vector; > + string_vector env; > pid_t pid; > > assert (!h->sock); > @@ -156,6 +155,7 @@ CONNECT_SA.START: > > if (prepare_socket_activation_environment (&env) == -1) { > SET_NEXT_STATE (%.DEAD); > + set_error (errno, "prepare_socket_activation_environment"); Why move this out of the function?