On Fri, Jun 16, 2023 at 07:14:01AM +0200, Theo Buehler wrote:
> With the last print_host() contortions out of the way, this is only used
> via print_addr() and can go. Make sa, buf, len local. Align variables.
> Unindent the if (buf == NULL) path.

One minor comment below.
 
> Index: iked.h
> ===================================================================
> RCS file: /cvs/src/sbin/iked/iked.h,v
> retrieving revision 1.216
> diff -u -p -r1.216 iked.h
> --- iked.h    13 Jun 2023 12:34:12 -0000      1.216
> +++ iked.h    16 Jun 2023 04:58:59 -0000
> @@ -1251,8 +1251,6 @@ struct in6_addr *
>  uint32_t
>        prefixlen2mask(uint8_t);
>  const char *
> -      print_host(struct sockaddr *, char *, size_t);
> -const char *
>        print_addr(void *);
>  char *get_string(uint8_t *, size_t);
>  const char *
> Index: util.c
> ===================================================================
> RCS file: /cvs/src/sbin/iked/util.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 util.c
> --- util.c    13 Jun 2023 12:34:12 -0000      1.41
> +++ util.c    14 Jun 2023 14:33:54 -0000
> @@ -636,19 +636,20 @@ prefixlen2mask6(uint8_t prefixlen, uint3
>  }
>  
>  const char *
> -print_host(struct sockaddr *sa, char *buf, size_t len)
> +print_addr(void *addr)
>  {
> -     static char     sbuf[IKED_CYCLE_BUFFERS][NI_MAXHOST + 7];
> -     static int      idx = 0;
> -     char            pbuf[7];
> -     in_port_t       port;
> -
> -     if (buf == NULL) {
> -             buf = sbuf[idx];
> -             len = sizeof(sbuf[idx]);
> -             if (++idx >= IKED_CYCLE_BUFFERS)
> -                     idx = 0;
> -     }
> +     struct sockaddr *sa = addr;
> +     static char      sbuf[IKED_CYCLE_BUFFERS][NI_MAXHOST + 7];
> +     char            *buf;
> +     size_t           len;
> +     static int       idx = 0;
> +     char             pbuf[7];
> +     in_port_t        port;

I would prefer if you keep the two static variables together and at the
top. Also no need to initialize idx to 0 (not sure if compilers are
finally smart enough to use bss for that and not the data segment).

With that changed, OK claudio@

> +
> +     buf = sbuf[idx];
> +     len = sizeof(sbuf[idx]);
> +     if (++idx >= IKED_CYCLE_BUFFERS)
> +             idx = 0;
>  
>       if (sa->sa_family == AF_UNSPEC) {
>               strlcpy(buf, "any", len);
> @@ -667,12 +668,6 @@ print_host(struct sockaddr *sa, char *bu
>       }
>  
>       return (buf);
> -}
> -
> -const char *
> -print_addr(void *addr)
> -{
> -     return print_host(addr, NULL, 0);
>  }
>  
>  char *
> 

-- 
:wq Claudio

Reply via email to