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.fu...@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
