correct reading/writing of complete unix timestamps

[blole]

Previously timestamps saved with SetNV() were truncated to two decimal places:
1472670993.01
1472670993.012345

Additionally they were also loaded as a double from GetNV() instead of as
two longs, and this opened up for comparison errors and meant that when
playing back buffers from private queries, the last query was always replayed
if it had a certain timestamp, which it would half of the time. ;)

This addresses issue #6

[Jay2k1]

Oh my god. You are my hero. Thank you!

[ghost]

Awesome, thank you!

[CyberShadow]

According to man timercmp, this might be unportable. Consider instead return !timercmp(&seen, &tv, <);

[CyberShadow]

This seems to assume that time_t is 64-bit.

[CyberShadow]

Thank you, confirmed that this patch works! Aside from the portability nits, LGTM :)

[michalrus]

So there’s this one slight issue left… When using znc.in/self-message capability, after reconnecting, my own last message is sent back to me. But only to the client that actually sent it, others work okay.

[Jay2k1]

Yup, I have this issue too.

[michalrus]

znc.in/self-message is a better privmsg.so, I wonder if @jpnurmi had it available, when writing clientbuffer.so.

My first guess would be that sending a message from a client (identifier) does not update its seen timestamp. This would cause the issue.

Let me read into the code…

[michalrus]

Oh, it seems like it’s more complicated. So the timestamp of me sending the PRIVMSG is indeed updated. But the timestamp of this entry in the buffer is slightly off. 300 µs in my case.

Hmmmmmmmmmmmmm. Now, that’s a bummer. What do?

[michalrus]

Similar difference (about 300 µs) is happening with channel buffers. But, somehow, we’re not getting those again. From quick look at znc’s source code, I can’t really tell what’s the difference. Both query and channel AddBuffer take ts == NULL (which is consistent with the observed difference of 300 µs).

[michalrus]

Frankly, I dunno. Until we get something better, here’s a very silly/naïve one, that I’m not proud off… Adding 500 µs to every timestamp. 😞

diff --git a/clientbuffer.cpp b/clientbuffer.cpp
index 7a9079a..3054af3 100644
--- a/clientbuffer.cpp
+++ b/clientbuffer.cpp
@@ -307,6 +307,7 @@ void CClientBufferMod::UpdateTimestamp(const CClient* client, const CString& tar
         if (HasClient(identifier)) {
             timeval tv;
             gettimeofday(&tv, NULL);
+            tv.tv_usec += 500;
             SetTimestamp(identifier, target, tv);
         }
     }

[Jay2k1]

How did you measure the timestamp difference? I could check this with my setup if I knew how.

[michalrus]

@Jay2k1, here:

diff --git a/clientbuffer.cpp b/clientbuffer.cpp
index 7a9079a..1432b3a 100644
--- a/clientbuffer.cpp
+++ b/clientbuffer.cpp
@@ -270,6 +270,7 @@ bool CClientBufferMod::SetTimestamp(const CString& identifier, const CString& ta
 {
     char timestamp[32];
     std::snprintf(timestamp, 32, "%ld.%06ld", tv.tv_sec, tv.tv_usec);
+    PutModule("→ SetTimestamp(" + identifier + ", " + target + ", " + timestamp + ")");
     return SetNV(identifier + "/" + target, timestamp);
 }

It’s more like a rough estimate… But connect with two clients, start a query with someone else, and compare timestamps of your own message between the two connected clients.

Nb. this += 500 µs would also fix #6 on its own. But this is sooooo uglyyyyyy. 😞

[Jay2k1]

OK, did that, results:

→ SetTimestamp(sending-client, target, 1481239139.996601)
→ SetTimestamp(another-client, target, 1481239139.997203)

I'd like to help think, but I don't understand enough of znc and the language to actually understand how the module works...

[michalrus]

So it’s 600 µs in your case. That’s exactly why my solution is nonsensical and just duct tape. 😜

Each message/line in ZNC, from beginning to end, should have a single timestamp associated with it and accessible from within modules. /cc @psychon

[psychon]

Each message/line in ZNC, from beginning to end, should have a single timestamp associated with it and accessible from within modules. /cc @psychon

Sounds like a good idea. An implementation idea would be to add this to CMessage. If I understand this right, every IRC-line that ZNC parses ends up as an instance of someting inheriting from CMessage, so if CMessage took a timestamp e.g. when it is constructed, everything would have access to a consistent timestamp.

Done. I took a look just as requested. :-P

[michalrus]

Uh-huh. =) TY! I’ll copy this to the ZNC repo, too.

[CyberShadow]

@blole: FYI, I've integrated this patch into my fork, which is now the project's main repository (as @jpnurmi hasn't been maintaining this one), so feel free to close this PR here. Thanks!

[blole]

Aha, great 😄