[Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)

Michal Domonkos notifications at github.com
Wed Oct 30 17:17:54 UTC 2024


@dmnks commented on this pull request.

A couple of questions:

1. We already know what messages (code & string) have been emitted in the log context as they're stored in the `recs` member (currently a vector). Is there any benefit of adding a new `seen` map, instead of turning `recs` into such a map, adding a counter to `rpmlogRec_s` and using that?
2. Is there any use case for keying on an additional string rather than on the actual log message?
3. Once we support non-global logging contexts, the `domain` would no longer be needed. That is, all users/domains (e.g. `ts`) would get a separate log context, instead of tracking them centrally in the global context. Is that correct?

I see that this is meant to be a PoC kind of thing, and as such it looks fine (some comments inline).

@pmatilai, any other opinion?

> @@ -412,3 +415,43 @@ void rpmlog (int code, const char *fmt, ...)
 exit:
     errno = saved_errno;
 }
+
+int rpmlogOnce (uint64_t domain, const char * key, int code, const char *fmt, ...)
+{
+    int saved_errno = errno;
+    rpmlogCtx ctx = rpmlogCtxAcquire();
+    int newkey = 0;
+
+    if (ctx) {
+	wrlock lock(ctx->mutex);
+	newkey = !ctx->seen[domain][{code, key}]++;

Showcases the power of C++, nicely done! :smile: 

> @@ -412,3 +415,43 @@ void rpmlog (int code, const char *fmt, ...)
 exit:
     errno = saved_errno;
 }
+
+int rpmlogOnce (uint64_t domain, const char * key, int code, const char *fmt, ...)
+{
+    int saved_errno = errno;
+    rpmlogCtx ctx = rpmlogCtxAcquire();
+    int newkey = 0;
+
+    if (ctx) {

Is this check necessary? We don't seem to be doing it elsewhere in this file (the global `ctx` is always returned).

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3417#pullrequestreview-2405628199
You are receiving this because you are subscribed to this thread.

Message ID: <rpm-software-management/rpm/pull/3417/review/2405628199 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20241030/01742396/attachment.html>


More information about the Rpm-maint mailing list