From: Lukasz Majewski <lukma@denx.de>
To: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Cc: Alistair Francis <alistair.francis@wdc.com>, libc-alpha@sourceware.org
Subject: Re: [PATCH 2/5] login: Move gnu utmpx to default implementaion
Date: Thu, 22 Oct 2020 10:15:10 +0200 [thread overview]
Message-ID: <20201022101510.076bbf41@jawa> (raw)
In-Reply-To: <20200729205117.2925113-2-adhemerval.zanella@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 11956 bytes --]
Hi Adhemerval,
> It removes one indirection where generic implementation assumes that
> utmp and utmpx might differ and allows the optimize the symbol alias
> where getutmp is the same as getumpx.
>
> Checked on all afftected ABIs.
> ---
> {sysdeps/gnu/bits => bits}/struct_utmpx.h | 0
> {sysdeps/gnu/bits => bits}/utmpx.h | 0
> include/utmpx.h | 1 +
> login/getutmp.c | 34 ++++++++++++++++--
> login/getutmpx.c | 35
> +------------------ login/updwtmp.c |
> 11 +++++- {sysdeps/gnu => login}/utmpx.h | 0
> sysdeps/gnu/getutmp.c | 34 ------------------
> sysdeps/gnu/getutmpx.c | 1 -
> sysdeps/gnu/updwtmp.c | 31 ----------------
> .../unix/sysv/linux/s390/s390-32/getutmp.c | 21 ++++-------
> .../unix/sysv/linux/s390/s390-32/updwtmp.c | 2 +-
> 12 files changed, 52 insertions(+), 118 deletions(-)
> rename {sysdeps/gnu/bits => bits}/struct_utmpx.h (100%)
> rename {sysdeps/gnu/bits => bits}/utmpx.h (100%)
> create mode 100644 include/utmpx.h
> rename {sysdeps/gnu => login}/utmpx.h (100%)
> delete mode 100644 sysdeps/gnu/getutmp.c
> delete mode 100644 sysdeps/gnu/getutmpx.c
> delete mode 100644 sysdeps/gnu/updwtmp.c
Nice that we would have the getutmp.c in one place.
Reviewed-by: Lukasz Majewski <lukma@denx.de>
>
> diff --git a/sysdeps/gnu/bits/struct_utmpx.h b/bits/struct_utmpx.h
> similarity index 100%
> rename from sysdeps/gnu/bits/struct_utmpx.h
> rename to bits/struct_utmpx.h
> diff --git a/sysdeps/gnu/bits/utmpx.h b/bits/utmpx.h
> similarity index 100%
> rename from sysdeps/gnu/bits/utmpx.h
> rename to bits/utmpx.h
> diff --git a/include/utmpx.h b/include/utmpx.h
> new file mode 100644
> index 0000000000..cfe9b7c054
> --- /dev/null
> +++ b/include/utmpx.h
> @@ -0,0 +1 @@
> +#include <login/utmpx.h>
> diff --git a/login/getutmp.c b/login/getutmp.c
> index e9a5fe69a5..3058a93d1b 100644
> --- a/login/getutmp.c
> +++ b/login/getutmp.c
> @@ -17,17 +17,47 @@
>
> #include <string.h>
> #include <utmp.h>
> +#include <stddef.h>
> +#define getutmpx __redirect_getutmpx
> #include <utmpx.h>
> +#undef getutmpx
> +
> +#define CHECK_SIZE_AND_OFFSET(field) \
> + _Static_assert (sizeof ((struct utmp){0}.field) \
> + == sizeof ((struct utmpx){0}.field),
> \
> + "sizeof ((struct utmp){0}." #field " != " \
> + "sizeof ((struct utmpx){0}" #field); \
> + _Static_assert (offsetof (struct utmp, field)
> \
> + == offsetof (struct utmpx, field), \
> + "offsetof (struct utmp, " #field ") != " \
> + "offsetof (struct utmpx, " #field ")");
> +
> +/* This ensure the getutmp to getutmpx alias is valid. */
> +_Static_assert (sizeof (struct utmp) == sizeof (struct utmpx),
> + "sizeof (struct utmp) != sizeof (struct utmpx)");
> +CHECK_SIZE_AND_OFFSET (ut_type)
> +CHECK_SIZE_AND_OFFSET (ut_pid)
> +CHECK_SIZE_AND_OFFSET (ut_line)
> +CHECK_SIZE_AND_OFFSET (ut_user)
> +CHECK_SIZE_AND_OFFSET (ut_id)
> +CHECK_SIZE_AND_OFFSET (ut_host)
> +CHECK_SIZE_AND_OFFSET (ut_tv)
> +
>
> /* Copy the information in UTMPX to UTMP. */
> void
> -getutmp (const struct utmpx *utmpx, struct utmp *utmp)
> +__getutmp (const struct utmpx *utmpx, struct utmp *utmp)
> {
> + memset (utmp, 0, sizeof (struct utmpx));
> utmp->ut_type = utmpx->ut_type;
> utmp->ut_pid = utmpx->ut_pid;
> memcpy (utmp->ut_line, utmpx->ut_line, sizeof (utmp->ut_line));
> memcpy (utmp->ut_user, utmpx->ut_user, sizeof (utmp->ut_user));
> memcpy (utmp->ut_id, utmpx->ut_id, sizeof (utmp->ut_id));
> memcpy (utmp->ut_host, utmpx->ut_host, sizeof (utmp->ut_host));
> - utmp->ut_tv = utmpx->ut_tv;
> + utmp->ut_tv.tv_sec = utmpx->ut_tv.tv_sec;
> + utmp->ut_tv.tv_usec = utmpx->ut_tv.tv_usec;
> }
> +
> +weak_alias (__getutmp, getutmp)
> +strong_alias (__getutmp, getutmpx)
> diff --git a/login/getutmpx.c b/login/getutmpx.c
> index 250a355235..839eb6826e 100644
> --- a/login/getutmpx.c
> +++ b/login/getutmpx.c
> @@ -1,34 +1 @@
> -/* Copyright (C) 1999-2020 Free Software Foundation, Inc.
> - This file is part of the GNU C Library.
> -
> - The GNU C Library is free software; you can redistribute it and/or
> - modify it under the terms of the GNU Lesser General Public
> - License as published by the Free Software Foundation; either
> - version 2.1 of the License, or (at your option) any later version.
> -
> - The GNU C Library is distributed in the hope that it will be
> useful,
> - but WITHOUT ANY WARRANTY; without even the implied warranty of
> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> - Lesser General Public License for more details.
> -
> - You should have received a copy of the GNU Lesser General Public
> - License along with the GNU C Library; if not, see
> - <https://www.gnu.org/licenses/>. */
> -
> -#include <string.h>
> -#include <utmp.h>
> -#include <utmpx.h>
> -
> -/* Copy the information in UTMP to UTMPX. */
> -void
> -getutmpx (const struct utmp *utmp, struct utmpx *utmpx)
> -{
> - memset (utmpx, 0, sizeof (struct utmpx));
> - utmpx->ut_type = utmp->ut_type;
> - utmpx->ut_pid = utmp->ut_pid;
> - memcpy (utmpx->ut_line, utmp->ut_line, sizeof (utmp->ut_line));
> - memcpy (utmpx->ut_user, utmp->ut_user, sizeof (utmp->ut_user));
> - memcpy (utmpx->ut_id, utmp->ut_id, sizeof (utmp->ut_id));
> - memcpy (utmpx->ut_host, utmp->ut_host, sizeof (utmp->ut_host));
> - utmpx->ut_tv = utmp->ut_tv;
> -}
> +/* Implemented by getutmp.c. */
> diff --git a/login/updwtmp.c b/login/updwtmp.c
> index f6cd515ac4..489c28b553 100644
> --- a/login/updwtmp.c
> +++ b/login/updwtmp.c
> @@ -17,11 +17,20 @@
> <https://www.gnu.org/licenses/>. */
>
> #include <utmp.h>
> +#include <string.h>
> +#include <unistd.h>
>
> #include "utmp-private.h"
>
> #ifndef TRANSFORM_UTMP_FILE_NAME
> -# define TRANSFORM_UTMP_FILE_NAME(file_name) (file_name)
> +# define TRANSFORM_UTMP_FILE_NAME(file_name) \
> + ((strcmp (file_name, _PATH_UTMP "x") == 0 \
> + && __access (_PATH_UTMP "x", F_OK) != 0) \
> + ? _PATH_UTMP \
> + : ((strcmp (file_name, _PATH_WTMP "x") == 0 \
> + && __access (_PATH_WTMP "x", F_OK) != 0) \
> + ? _PATH_WTMP \
> + : file_name))
> #endif
>
> void
> diff --git a/sysdeps/gnu/utmpx.h b/login/utmpx.h
> similarity index 100%
> rename from sysdeps/gnu/utmpx.h
> rename to login/utmpx.h
> diff --git a/sysdeps/gnu/getutmp.c b/sysdeps/gnu/getutmp.c
> deleted file mode 100644
> index 95a9a4b354..0000000000
> --- a/sysdeps/gnu/getutmp.c
> +++ /dev/null
> @@ -1,34 +0,0 @@
> -/* Copyright (C) 1999-2020 Free Software Foundation, Inc.
> - This file is part of the GNU C Library.
> -
> - The GNU C Library is free software; you can redistribute it and/or
> - modify it under the terms of the GNU Lesser General Public
> - License as published by the Free Software Foundation; either
> - version 2.1 of the License, or (at your option) any later version.
> -
> - The GNU C Library is distributed in the hope that it will be
> useful,
> - but WITHOUT ANY WARRANTY; without even the implied warranty of
> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> - Lesser General Public License for more details.
> -
> - You should have received a copy of the GNU Lesser General Public
> - License along with the GNU C Library; if not, see
> - <https://www.gnu.org/licenses/>. */
> -
> -#include <assert.h>
> -#include <string.h>
> -#include <utmp.h>
> -#ifndef _UTMPX_H
> -/* This is an ugly hack but we must not see the getutmpx
> declaration. */ -# define getutmpx XXXgetutmpx
> -# include <utmpx.h>
> -# undef getutmpx
> -#endif
> -
> -void
> -getutmp (const struct utmpx *utmpx, struct utmp *utmp)
> -{
> - assert (sizeof (struct utmp) == sizeof (struct utmpx));
> - memcpy (utmp, utmpx, sizeof (struct utmp));
> -}
> -strong_alias (getutmp, getutmpx)
> diff --git a/sysdeps/gnu/getutmpx.c b/sysdeps/gnu/getutmpx.c
> deleted file mode 100644
> index f393734a63..0000000000
> --- a/sysdeps/gnu/getutmpx.c
> +++ /dev/null
> @@ -1 +0,0 @@
> -/* We don't need a separate version. it is the same as getutmp().
> */ diff --git a/sysdeps/gnu/updwtmp.c b/sysdeps/gnu/updwtmp.c
> deleted file mode 100644
> index 044091b77c..0000000000
> --- a/sysdeps/gnu/updwtmp.c
> +++ /dev/null
> @@ -1,31 +0,0 @@
> -/* Copyright (C) 1998-2020 Free Software Foundation, Inc.
> - This file is part of the GNU C Library.
> - Contributed by Mark Kettenis <kettenis@phys.uva.nl>, 1998.
> -
> - The GNU C Library is free software; you can redistribute it and/or
> - modify it under the terms of the GNU Lesser General Public
> - License as published by the Free Software Foundation; either
> - version 2.1 of the License, or (at your option) any later version.
> -
> - The GNU C Library is distributed in the hope that it will be
> useful,
> - but WITHOUT ANY WARRANTY; without even the implied warranty of
> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> - Lesser General Public License for more details.
> -
> - You should have received a copy of the GNU Lesser General Public
> - License along with the GNU C Library; if not, see
> - <https://www.gnu.org/licenses/>. */
> -
> -#include <string.h>
> -#include <unistd.h>
> -
> -#define TRANSFORM_UTMP_FILE_NAME(file_name) \
> - ((strcmp (file_name, _PATH_UTMP "x") == 0 \
> - && __access (_PATH_UTMP "x", F_OK) != 0) \
> - ? _PATH_UTMP \
> - : ((strcmp (file_name, _PATH_WTMP "x") == 0 \
> - && __access (_PATH_WTMP "x", F_OK) != 0) \
> - ? _PATH_WTMP \
> - : file_name))
> -
> -#include <login/updwtmp.c>
> diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/getutmp.c
> b/sysdeps/unix/sysv/linux/s390/s390-32/getutmp.c index
> 6380ae2a82..50ce8c4f5c 100644 ---
> a/sysdeps/unix/sysv/linux/s390/s390-32/getutmp.c +++
> b/sysdeps/unix/sysv/linux/s390/s390-32/getutmp.c @@ -16,22 +16,15 @@
> License along with the GNU C Library; if not, see
> <https://www.gnu.org/licenses/>. */
>
> -#include <string.h>
> -#include <utmp.h>
> -/* This is an ugly hack but we must not see the getutmpx
> declaration. */ -#define getutmpx XXXgetutmpx
> -#include <utmpx.h>
> -#undef getutmpx
> +#undef weak_alias
> +#define weak_alias(a, b)
> +#undef strong_alias
> +#define strong_alias(a, b)
>
> -#include "utmp-compat.h"
> +#include <login/getutmp.c>
>
> -#undef weak_alias
> -#define weak_alias(n,a)
> -#define getutmp __getutmp
> -#define getutmpx __getutmpx
> -#include "sysdeps/gnu/getutmp.c"
> -#undef getutmp
> -#undef getutmpx
> +#include "utmp-compat.h"
>
> default_symbol_version (__getutmp, getutmp, UTMP_COMPAT_BASE);
> +_strong_alias (__getutmp, __getutmpx)
> default_symbol_version (__getutmpx, getutmpx, UTMP_COMPAT_BASE);
> diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/updwtmp.c
> b/sysdeps/unix/sysv/linux/s390/s390-32/updwtmp.c index
> dcd334e5a9..2079571cc1 100644 ---
> a/sysdeps/unix/sysv/linux/s390/s390-32/updwtmp.c +++
> b/sysdeps/unix/sysv/linux/s390/s390-32/updwtmp.c @@ -25,7 +25,7 @@
> # undef weak_alias
> # define weak_alias(n,a)
> #endif
> -#include "sysdeps/gnu/updwtmp.c"
> +#include <login/updwtmp.c>
>
> #if defined SHARED
> default_symbol_version (__updwtmp, updwtmp, UTMP_COMPAT_BASE);
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2020-10-22 8:16 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-29 20:51 [PATCH 1/5] login: Consolidate utmp and utmpx headers Adhemerval Zanella via Libc-alpha
2020-07-29 20:51 ` [PATCH 2/5] login: Move gnu utmpx to default implementaion Adhemerval Zanella via Libc-alpha
2020-10-22 8:15 ` Lukasz Majewski [this message]
2020-10-22 9:16 ` Andreas Schwab
2020-07-29 20:51 ` [PATCH 3/5] login: Add 64-bit time support Adhemerval Zanella via Libc-alpha
2020-07-29 21:17 ` Joseph Myers
2020-07-30 12:34 ` Adhemerval Zanella via Libc-alpha
2020-08-02 19:02 ` Maciej W. Rozycki via Libc-alpha
2020-08-02 22:05 ` Adhemerval Zanella via Libc-alpha
2020-10-22 9:16 ` Lukasz Majewski
2020-07-29 20:51 ` [PATCH 4/5] login: User 64-bit time on struct lastlog Adhemerval Zanella via Libc-alpha
2020-07-29 21:04 ` Andreas Schwab
2020-07-29 21:14 ` Andreas Schwab
2020-07-30 12:39 ` Adhemerval Zanella via Libc-alpha
2020-07-30 16:19 ` Florian Weimer via Libc-alpha
2020-07-30 18:54 ` Joseph Myers
2020-07-30 21:53 ` Adhemerval Zanella via Libc-alpha
2020-07-31 0:31 ` Joseph Myers
2020-07-30 21:46 ` Adhemerval Zanella via Libc-alpha
2020-10-22 9:25 ` Lukasz Majewski
2020-07-29 20:51 ` [PATCH 5/5] Remove __WORDSIZE_TIME64_COMPAT32 Adhemerval Zanella via Libc-alpha
2020-10-22 9:31 ` Lukasz Majewski
2020-07-29 21:08 ` [PATCH 1/5] login: Consolidate utmp and utmpx headers Joseph Myers
2020-07-30 12:36 ` Adhemerval Zanella via Libc-alpha
2020-10-22 8:06 ` Lukasz Majewski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/libc/involved.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201022101510.076bbf41@jawa \
--to=lukma@denx.de \
--cc=adhemerval.zanella@linaro.org \
--cc=alistair.francis@wdc.com \
--cc=libc-alpha@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).