From: "Trapp, Michael" <michael.trapp@sap.com>
To: Karel Zak <kzak@redhat.com>
Cc: "util-linux@vger.kernel.org" <util-linux@vger.kernel.org>
Subject: Re: [PATCH] uuidd: add cont_clock persistence
Date: Wed, 10 Jan 2024 15:42:33 +0000 [thread overview]
Message-ID: <6EFE3540-1C52-48E7-8E18-6C199DB6823F@sap.com> (raw)
In-Reply-To: <20240104121230.3bi7nt52tb3fu6ww@ws.net.home>
Hi Karel,
> On 4. Jan 2024, at 13:12, Karel Zak <kzak@redhat.com> wrote:
>
> On Fri, Dec 15, 2023 at 11:18:29PM +0100, Michael Trapp wrote:
>> cont_clock requires a correct time setup and therefore it
>> must be possible to detect a step back between uuidd starts.
>> ...
>> + enum { fd_init = -2, fd_error = -1 };
>
> In the code (below) the enum items seems like variables, a little bit
> confusing. It would be better use upper-case, STATE_FD_INIT, STATE_FD_ERROR.
>> + static int state_fd = fd_init;
>> ...
>> + if (state_fd == fd_error)
Makes sense and will be fixed.
>> ...
>> + save_umask = umask(0);
>> + state_fd = open(LIBUUID_CLOCK_CONT_FILE, O_RDWR|O_CREAT|O_CLOEXEC, 0660);
>> + (void) umask(save_umask);
>> + if (state_fd == fd_error)
>> + return -1;
>> +
>> + state_f = fdopen(state_fd, "r+" UL_CLOEXECSTR);
>> + if (!state_f)
>> + goto error;
>
> Seems it duplicates code from get_clock(), what about introduce a generic
>
> state_fd_init(LIBUUID_CLOCK_CONT_FILE, &state_fd, &state_f);
>
> and use the same in get_clock() for LIBUUID_CLOCK_FILE?
That’s the case for the initialization with open()/fdopen().
I’ll move this into the new function state_fd_init().
>> + if (fstat(state_fd, &st))
>> + goto error;
>> +
>> + if (st.st_size) {
>> + rewind(state_f);
>> + if (fscanf(state_f, "cont: %lu\n", &last_clock_reg) != 1)
>> + goto error;
>> + } else
>> + last_clock_reg = clock_reg;
>
> For LIBUUID_CLOCK_FILE we use flock(), I guess it's unnecessary for
> LIBUUID_CLOCK_CONT_FILE as we assume only one uuidd instance, right?
That’s correct, based on the check in uuidd.c, there can't be multiple instances.
Michael
prev parent reply other threads:[~2024-01-10 15:42 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-15 22:18 [PATCH] uuidd: add cont_clock persistence Michael Trapp
2024-01-04 12:12 ` Karel Zak
2024-01-10 15:42 ` Trapp, Michael [this message]
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6EFE3540-1C52-48E7-8E18-6C199DB6823F@sap.com \
--to=michael.trapp@sap.com \
--cc=kzak@redhat.com \
--cc=util-linux@vger.kernel.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).