On 1/30/23 23:55, Richard W.M. Jones wrote:
> Some small refactorings which should not affect the code:
>
> - Use string_vector_reserve instead of checking each time we append.
>
> - Get rid of the hard-coded length, and use strncmp (..., s, strlen (s)).
> The compiler should compile this to the same code.
> ---
> generator/states-connect-socket-activation.c | 23 +++++++-------------
> 1 file changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/generator/states-connect-socket-activation.c
> b/generator/states-connect-socket-activation.c
> index 9a83834915..24544018fb 100644
> --- a/generator/states-connect-socket-activation.c
> +++ b/generator/states-connect-socket-activation.c
> @@ -34,9 +34,6 @@
> /* This is baked into the systemd socket activation API. */
> #define FIRST_SOCKET_ACTIVATION_FD 3
>
> -/* == strlen ("LISTEN_PID=") | strlen ("LISTEN_FDS=") */
> -#define PREFIX_LENGTH 11
> -
> extern char **environ;
>
> /* Prepare environment for calling execvp when doing systemd socket
> @@ -53,26 +50,22 @@ prepare_socket_activation_environment (string_vector *env)
>
> assert (env->len == 0);
>
> - /* Reserve slots env[0] and env[1]. */
> + /* Reserve slots env[0]..env[1] */
> + if (string_vector_reserve (env, 2) == -1)
> + goto err;
> p = strdup ("LISTEN_PID=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx");
> if (p == NULL)
> goto err;
> - if (string_vector_append (env, p) == -1) {
> - free (p);
> - goto err;
> - }
> + string_vector_append (env, p);
> p = strdup ("LISTEN_FDS=1");
> if (p == NULL)
> goto err;
> - if (string_vector_append (env, p) == -1) {
> - free (p);
> - goto err;
> - }
> + string_vector_append (env, p);
>
> /* Append the current environment, but remove LISTEN_PID, LISTEN_FDS. */
> for (i = 0; environ[i] != NULL; ++i) {
> - if (strncmp (environ[i], "LISTEN_PID=", PREFIX_LENGTH) != 0 &&
> - strncmp (environ[i], "LISTEN_FDS=", PREFIX_LENGTH) != 0) {
> + if (strncmp (environ[i], "LISTEN_PID=", strlen ("LISTEN_PID=")) != 0 &&
> + strncmp (environ[i], "LISTEN_FDS=", strlen ("LISTEN_FDS=")) != 0) {
> char *copy = strdup (environ[i]);
> if (copy == NULL)
> goto err;
> @@ -194,7 +187,7 @@ CONNECT_SA.START:
> char buf[32];
> const char *v =
> nbd_internal_fork_safe_itoa ((long) getpid (), buf, sizeof buf);
> - strcpy (&env.ptr[0][PREFIX_LENGTH], v);
> + strcpy (&env.ptr[0][strlen ("LISTEN_FDS=")], v);
>
> /* Restore SIGPIPE back to SIG_DFL. */
> signal (SIGPIPE, SIG_DFL);
I'd propose a more comprehensive refactoring here, see under my v1 comments.
Laszlo
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
