Laszlo Ersek
2023-Feb-15 14:11 UTC
[Libguestfs] [libnbd PATCH v3 12/29] socket activation: clean up responsibilities of prep.sock.act.env.()
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 at redhat.com>
---
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;
/* 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");
close (s);
return 0;
}
Richard W.M. Jones
2023-Feb-15 16:39 UTC
[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 at redhat.com> > --- > 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? Rich.> close (s); > return 0; > } > > _______________________________________________ > Libguestfs mailing list > Libguestfs at redhat.com > https://listman.redhat.com/mailman/listinfo/libguestfs-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v