Re: syslogd: Adjust to readutmp changes in Gnulib.

[Erik Auerswald]

Hi Collin,

On Wed, Sep 04, 2024 at 07:36:25PM -0700, Collin Funk wrote:
> [...]
> I don't use syslogd much but I wrote the attached patch and did some
> verify minimal testing with 'syslogd --debug --rcfile' and some basic
> rules.

I'd say that the important tests are to log to all users, and to log to
a specific user, because that functionality seems to read utmp entries.

> Can someone give it a look over before pushing? I'll write a patch for
> talkd later.

I'll take a look, but I am not familiar with the syslogd code or reading
utmp entries.  So I'll just ask questions regarding some changes.

> >From cbc4d8289f4c5c034715302deb3be7e431cb7fb8 Mon Sep 17 00:00:00 2001
> From: Collin Funk <collin.funk1@gmail.com>
> Date: Wed, 4 Sep 2024 19:30:55 -0700
> Subject: [PATCH] syslogd: Adjust to readutmp changes in Gnulib.
> 
> * src/syslogd.c (wallmsg): Let readutmp handle portability. Don't use
> sizeof on a pointer.
> ---
>  src/syslogd.c | 51 +++++++++++++++++++--------------------------------
>  1 file changed, 19 insertions(+), 32 deletions(-)
> 
> diff --git a/src/syslogd.c b/src/syslogd.c
> index 6662e8a0..8fcada80 100644
> --- a/src/syslogd.c
> +++ b/src/syslogd.c
> @@ -1618,35 +1618,25 @@ void
>  wallmsg (struct filed *f, struct iovec *iov)
>  {
>    static int reenter;                /* Avoid calling ourselves.  */
> -  STRUCT_UTMP *utp;
> -#if defined UTMP_NAME_FUNCTION || !defined HAVE_GETUTXENT
> -  STRUCT_UTMP *utmpbuf;
> -  size_t utmp_count;
> -#endif /* UTMP_NAME_FUNCTION || !HAVE_GETUTXENT */
> -  int i;
> +  struct gl_utmp *utp;
> +  struct gl_utmp *utmpbuf;
> +  idx_t utmp_count;

This reflects the Gnulib API change, right?

>    char *p;
> -  char line[sizeof (utp->ut_line) + 1];

This was used to store a copy of utp->ut_line.  Your patch removes this
copy and works directly with utp->ut_line.  Can you explain why this
code used a copy, and why this copy is no longer needed?

>    if (reenter++)
>      return;
>  
> -#if !defined UTMP_NAME_FUNCTION && defined HAVE_GETUTXENT
> -  setutxent ();
> -
> -  while ((utp = getutxent ()))
> -#else /* UTMP_NAME_FUNCTION || !HAVE_GETUTXENT */
>    if (read_utmp (UTMP_FILE, &utmp_count, &utmpbuf,
> -              READ_UTMP_USER_PROCESS | READ_UTMP_CHECK_PIDS) < 0)
> +              READ_UTMP_USER_PROCESS | READ_UTMP_CHECK_PIDS) != 0)
>      {
>        logerror ("opening utmp file");
>        return;
>      }
>  
>    for (utp = utmpbuf; utp < utmpbuf + utmp_count; utp++)
> -#endif /* UTMP_NAME_FUNCTION || !HAVE_GETUTXENT */
>      {
> -      strncpy (line, utp->ut_line, sizeof (utp->ut_line));
> -      line[sizeof (utp->ut_line)] = '\0';
> +      char *line = utp->ut_line;

This is the omitted string copy operation.

> +
>        if (f->f_type == F_WALL)
>       {
>         /* Note we're using our own version of ttymsg
> @@ -1661,24 +1651,21 @@ wallmsg (struct filed *f, struct iovec *iov)
>         continue;
>       }
>        /* Should we send the message to this user? */
> -      for (i = 0; i < f->f_un.f_user.f_nusers; i++)
> -     if (!strncmp (f->f_un.f_user.f_unames[i], UT_USER (utp),
> -                   sizeof (UT_USER (utp))))
> -       {
> -         p = inetutils_ttymsg (iov, IOVCNT, line, TTYMSGTIME);
> -         if (p != NULL)
> -           {
> -             errno = 0;      /* Already in message. */
> -             logerror (p);
> -           }
> -         break;
> -       }
> +      for (idx_t i = 0; i < f->f_un.f_user.f_nusers; i++)
> +     {
> +       if (strcmp (f->f_un.f_user.f_unames[i], UT_USER (utp)) == 0)

Your patch replaces strncmp() with strcmp().  What ensures that all
involved memory areas are NUL-terminated strings?

> +         {
> +           p = inetutils_ttymsg (iov, IOVCNT, line, TTYMSGTIME);
> +           if (p != NULL)
> +             {
> +               errno = 0;    /* Already in message. */
> +               logerror (p);
> +             }
> +           break;
> +         }
> +     }
>      }
> -#if defined UTMP_NAME_FUNCTION || !defined HAVE_GETUTXENT
>    free (utmpbuf);
> -#else /* !UTMP_NAME_FUNCTION && HAVE_GETUTXENT */
> -  endutxent ();
> -#endif
>    reenter = 0;
>  }
>  
> -- 
> 2.46.0

Thanks,
Erik