Re: [PATCH] Code simplification in pk_hserver_make_hyperlink

[Dan Čermák]

Tim Rühsen <address@hidden> writes:

> 2020-04-02 Tim Rühsen  <address@hidden>
>
>         * src/pk-hserver.c (pk_hserver_make_hyperlink):
>         Simplify malloc/strlen/strcpy/strcat with asprintf.
>         (pk_hserver_init): Remove writing to hserver_port_str.
>         (hserver_port_str): Remove variable.
> ---
>  ChangeLog        |  7 +++++++
>  src/pk-hserver.c | 47 +++++++++++++++--------------------------------
>  2 files changed, 22 insertions(+), 32 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index 5e9d0f7c..e44125a3 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,10 @@
> +2020-04-02 Tim Rühsen  <address@hidden>
> +
> +     * src/pk-hserver.c (pk_hserver_make_hyperlink):
> +     Simplify malloc/strlen/strcpy/strcat with asprintf.
> +     (pk_hserver_init): Remove writing to hserver_port_str.
> +     (hserver_port_str): Remove variable.
> +
>  2020-04-02 Tim Rühsen  <address@hidden>
>
>       * bootstrap.conf: Add gnulib module stdbool.
> diff --git a/src/pk-hserver.c b/src/pk-hserver.c
> index d65eb0f2..941c01dd 100644
> --- a/src/pk-hserver.c
> +++ b/src/pk-hserver.c
> @@ -50,7 +50,6 @@ static int hserver_socket;
>
>  /* Port where the server listens for connections.  */
>  static int hserver_port = 0;
> -static char hserver_port_str[128];
>
>  /* hserver_finish is used to tell the server threads to terminate.  It
>     is protected with a mutex.  */
> @@ -291,7 +290,6 @@ pk_hserver_init ()
>        exit (EXIT_FAILURE);
>      }
>    hserver_port = ntohs (clientname.sin_port);
> -  sprintf (hserver_port_str, "%d", hserver_port);
>
>    hserver_finish = 0;
>    ret = pthread_create (&hserver_thread,
> @@ -331,45 +329,30 @@ pk_hserver_make_hyperlink (char type,
>                             const char *cmd)
>  {
>    int token;
> -  char *str, token_str[128], type_str[2];
> +  char *str;
>    char hostname[128];
>
> +  if (gethostname (hostname, sizeof (hostname)) != 0)
> +    {
> +      perror ("gethostname");
> +      exit (EXIT_FAILURE);
> +    }
> +
>    assert (type == 'i' || type == 'e');
> -  type_str[0] = type;
> -  type_str[1] = '\0';
>
> -  /* XXX: check for maximum length 2048.  */
>    token = pk_hserver_get_token ();
> -  sprintf (token_str, "%d", token);
>
> -  if (gethostname (hostname, 128) != 0)
> +  if (asprintf(&str, "app://%s:%d/%d/%c/%s",
> +               hostname,
> +               hserver_port,
> +               token,
> +               type,
> +               cmd
> +               ) == -1)

Isn't asprintf a GNU extension that will make poke no longer portable to
other libc implementations beside glibc?

>      {
> -      perror ("gethostname");
> -      exit (EXIT_FAILURE);
> +      return NULL;
>      }
>
> -  str = xmalloc (strlen ("app://")
> -                 + strlen (hostname)
> -                 + 1 /* ':' */
> -                 + strlen (hserver_port_str)
> -                 + 1 /* '/' */
> -                 + strlen (token_str)
> -                 + 1 /* '/' */
> -                 + 1 /* type */
> -                 + 1 /* '/' */
> -                 + strlen (cmd)
> -                 + 1 /* '0' */);
> -  strcpy (str, "app://");
> -  strcat (str, hostname);
> -  strcat (str, ":");
> -  strcat (str, hserver_port_str);
> -  strcat (str, "/");
> -  strcat (str, token_str);
> -  strcat (str, "/");
> -  strcat (str, type_str);
> -  strcat (str, "/");
> -  strcat (str, cmd);
> -
>    return str;
>  }
>
> --
> 2.26.0

signature.asc
Description: PGP signature