Re: [PATCH v2] Change attachment stamping to use stat st_mtime by default.
Oswald Buddenhagen via Mutt-dev <[email protected]>
| Newsgroups | gmane.mail.mutt.devel |
|---|---|
| Message-ID | <[email protected]> |
On Fri, Aug 21, 2026 at 12:13:08PM +0800, Kevin J. McCarthy wrote:
>Recent Linux kernels have different clocks used for the time() system
>call and stat() st_mtime of a file.
actually, no clock is used for stat() at all.
>time() is set to use a lower granularity clock.
this sounds like something (mutt or at least some system config) is
actually setting it.
also, i'd generally speak of precision rather than granularity. to me,
the former seems more suggestive of general fuzziness, which seems more
fitting here.
>This means that
we're still in non-sequitur land here.
>right around the change of
>seconds, there can be an "X" millisecond gap where time() seconds is
>behind st_mtime seconds.
so my proposal would be to replace the paragraph with:
"
Modern operating systems offer multiple clocks of different precision.
Due to an implementation detail of the Linux kernel, there is a window
of a few milliseconds where the coarse clock's seconds may lag behind
the precise one's right after each change of seconds.
Recent Linux kernels use a precise clock for time-stamping files, while
the time() function we use as a reference has been using a coarse clock
for some time now.
"
(i made it as definite as possible, but please fact-check the
assertions.)
>The change has led to a bug in Mutt, where recording the stamp of an
>attachment using time() sometimes has a "second" value earlier than
>the stat() st_mtime of the file just modified (again, due to the
>granularity difference of the clock used).
>
>This caused a sporadic false warning before sending an email, that the
>attachment has changed on disk since it was last checked. For more
>details, see:
>https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1144613
>
>The attachment->stamp is only used to compare against the stat
>st_mtime of the file just before sending. So it actually makes more
>sense to just record the previous st_mtime of the file. This occurs
>during message composition, so an extra stat on a file that was just
>read is not likely to cause a meaningful slow down.
>
>Change mutt_stamp_attachment() to use st_mtime for the stamp.
>Since
>the function previously had no error case, use "time(NULL) + 1" as a
>fallback behavior in case stat fails or (for some strange reason) it's
>called for a receive-mode attachment.
>
this seems a bit sloppy at first sight, but i guess it's fine, as a
legitimate error (e.g., attachment disappearing or fs becoming
inaccessible) will be handled properly at a later point.
i'd still add a comment in the code to justify the fallback.
>Thanks to Vincent Lefèvre for reporting the issue and working on
>finding out the source of the bug. Thanks also to the other
>contributors in the thread who helped with reproducing and diagnosing
>the problem: Ian Collier, Reed Underwood, and Steffen Nurpmeso.
>---
>
>I've tried to make the commit message clearer. Please let me know if
>the explanation makes sense and my terminology is correct. If not,
>please do make suggestions. Thank you.
>
> sendlib.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
>diff --git a/sendlib.c b/sendlib.c
>index 150bc72d..0e4a8146 100644
>--- a/sendlib.c
>+++ b/sendlib.c
>@@ -1284,7 +1284,21 @@ static void mutt_set_encoding(BODY *b, CONTENT *info)
>
> void mutt_stamp_attachment(BODY *a)
> {
>- a->stamp = time(NULL);
>+ struct stat sb;
>+
>+ if (a->filename && stat(a->filename, &sb) == 0)
>+ a->stamp = sb.st_mtime;
>+ else
>+ {
>+ /* Recent Linux kernels have an issue where time()'s clock has a
>+ * lower granularity
"precision"
> than the stat st_mtime clock. This can
>+ * result in time()'s second value being earlier than the st_mtime
>+ * seconds of a file just modified. See:
>+ * https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1144613.
> As
>+ * an error fallback case,
>
this qualification seems misplaced here; see above instead.
>add 1 to work around the behavior.
>+ */
>+ a->stamp = time(NULL) + 1;
>+ }
> }
>
> /* Get a body's character set */
>--
>2.55.0
>