From: "Florian Weimer (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>
To: libc-alpha@sourceware.org
Cc: Florian Weimer <fweimer@redhat.com>
Subject: [review] login: Introduce matches_last_entry to utmp processing
Date: Tue, 12 Nov 2019 06:50:10 -0500 [thread overview]
Message-ID: <gerrit.1573559410000.Ib8a85002f7f87ee41590846d16d7e52bdb82f5a5@gnutoolchain-gerrit.osci.io> (raw)
In-Reply-To: gerrit.1573559410000.Ib8a85002f7f87ee41590846d16d7e52bdb82f5a5@gnutoolchain-gerrit.osci.io
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/614
......................................................................
login: Introduce matches_last_entry to utmp processing
This simplifies internal_getut_nolock and fixes a regression,
introduced in commit be6b16d975683e6cca57852cd4cfe715b2a9d8b1
("login: Acquire write lock early in pututline [BZ #24882]")
in pututxline because __utmp_equal can only compare process-related
utmp entries.
Fixes: be6b16d975683e6cca57852cd4cfe715b2a9d8b1
Change-Id: Ib8a85002f7f87ee41590846d16d7e52bdb82f5a5
---
M login/utmp_file.c
1 file changed, 31 insertions(+), 49 deletions(-)
diff --git a/login/utmp_file.c b/login/utmp_file.c
index 917c4c5..c41d17b 100644
--- a/login/utmp_file.c
+++ b/login/utmp_file.c
@@ -43,6 +43,25 @@
/* Cache for the last read entry. */
static struct utmp last_entry;
+/* Returns true if *ENTRY matches last_entry, based on
+ data->ut_type. */
+static bool
+matches_last_entry (const struct utmp *data)
+{
+ if (file_offset <= 0)
+ /* Nothing has been read. last_entry is stale and cannot match. */
+ return false;
+
+ if (data->ut_type == RUN_LVL
+ || data->ut_type == BOOT_TIME
+ || data->ut_type == OLD_TIME
+ || data->ut_type == NEW_TIME)
+ /* For some entry types, only a type match is required. */
+ return data->ut_type == last_entry.ut_type;
+ else
+ /* For the process-related entries, a full match is needed. */
+ return __utmp_equal (&last_entry, data);
+}
/* Locking timeout. */
#ifndef TIMEOUT
@@ -133,9 +152,6 @@
__lseek64 (file_fd, 0, SEEK_SET);
file_offset = 0;
- /* Make sure the entry won't match. */
- last_entry.ut_type = -1;
-
return 1;
}
@@ -191,48 +207,20 @@
static int
internal_getut_nolock (const struct utmp *id)
{
- if (id->ut_type == RUN_LVL || id->ut_type == BOOT_TIME
- || id->ut_type == OLD_TIME || id->ut_type == NEW_TIME)
+ while (1)
{
- /* Search for next entry with type RUN_LVL, BOOT_TIME,
- OLD_TIME, or NEW_TIME. */
-
- while (1)
+ /* Read the next entry. */
+ if (__read_nocancel (file_fd, &last_entry, sizeof (struct utmp))
+ != sizeof (struct utmp))
{
- /* Read the next entry. */
- if (__read_nocancel (file_fd, &last_entry, sizeof (struct utmp))
- != sizeof (struct utmp))
- {
- __set_errno (ESRCH);
- file_offset = -1l;
- return -1;
- }
- file_offset += sizeof (struct utmp);
-
- if (id->ut_type == last_entry.ut_type)
- break;
+ __set_errno (ESRCH);
+ file_offset = -1l;
+ return -1;
}
- }
- else
- {
- /* Search for the next entry with the specified ID and with type
- INIT_PROCESS, LOGIN_PROCESS, USER_PROCESS, or DEAD_PROCESS. */
+ file_offset += sizeof (struct utmp);
- while (1)
- {
- /* Read the next entry. */
- if (__read_nocancel (file_fd, &last_entry, sizeof (struct utmp))
- != sizeof (struct utmp))
- {
- __set_errno (ESRCH);
- file_offset = -1l;
- return -1;
- }
- file_offset += sizeof (struct utmp);
-
- if (__utmp_equal (&last_entry, id))
- break;
- }
+ if (matches_last_entry (id))
+ break;
}
return 0;
@@ -365,13 +353,7 @@
/* Find the correct place to insert the data. */
bool found = false;
- if (file_offset > 0
- && ((last_entry.ut_type == data->ut_type
- && (last_entry.ut_type == RUN_LVL
- || last_entry.ut_type == BOOT_TIME
- || last_entry.ut_type == OLD_TIME
- || last_entry.ut_type == NEW_TIME))
- || __utmp_equal (&last_entry, data)))
+ if (matches_last_entry (data))
{
if (__lseek64 (file_fd, file_offset, SEEK_SET) < 0)
{
@@ -389,7 +371,7 @@
found = false;
}
else
- found = __utmp_equal (&last_entry, data);
+ found = matches_last_entry (data);
}
if (!found)
--
Gerrit-Project: glibc
Gerrit-Branch: master
Gerrit-Change-Id: Ib8a85002f7f87ee41590846d16d7e52bdb82f5a5
Gerrit-Change-Number: 614
Gerrit-PatchSet: 1
Gerrit-Owner: Florian Weimer <fweimer@redhat.com>
Gerrit-MessageType: newchange
next parent reply other threads:[~2019-11-12 11:50 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-12 11:50 Florian Weimer (Code Review) [this message]
2019-11-12 16:07 ` [review] login: Introduce matches_last_entry to utmp processing Carlos O'Donell (Code Review)
2019-11-12 16:20 ` [pushed] " Sourceware to Gerrit sync (Code Review)
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=gerrit.1573559410000.Ib8a85002f7f87ee41590846d16d7e52bdb82f5a5@gnutoolchain-gerrit.osci.io \
--to=gerrit@gnutoolchain-gerrit.osci.io \
--cc=fweimer@redhat.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).