[Rpm-maint] [rpm-software-management/rpm] Check not configured keystore backends for keys (PR #3539)

Panu Matilainen notifications at github.com
Fri Jan 31 08:04:54 UTC 2025


@pmatilai commented on this pull request.



> @@ -71,6 +71,19 @@ rpmKeyring rpmKeyringFree(rpmKeyring keyring)
     return NULL;
 }
 
+size_t rpmKeyringSize(rpmKeyring keyring, int count_subkeys)
+{
+    if (!keyring) return 0;
+    rdlock lock(keyring->mutex);
+
+    size_t size = 0;
+    for (auto &pair : keyring->keys) {
+	if (count_subkeys or !pair.second->primarykey)

I didn't even know that `or` was legit c++ :flushed: I dunno what to think about that - it does look's weird and inconsistent in a codebase using `||` everywhere else.

Dunno if this kind of thing comes more naturally to you but I find this condition quite non-obvious and requiring extra processing, and that's never a good thing if it's easily avoidable. I'd write this something like
```
if (count_subkeys && pair.second->primarykey != NULL)
    continue;
++size;
```

That makes clear what primarykey is (ie, it's not a boolean that tells you whether this key is primary key or not, but almost the opposite) and does away with the non-obvious "or not".

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

Message ID: <rpm-software-management/rpm/pull/3539/review/2585919016 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20250131/ce7959a4/attachment.htm>


More information about the Rpm-maint mailing list