[Rpm-maint] [rpm-software-management/rpm] Extend dbus-announce plugin (PR #3532)

[Michal Domonkos]

@dmnks commented on this pull request.



> @@ -103,9 +119,77 @@ static rpmRC send_ts_message(rpmPlugin plugin,
 
     dbcookie = rpmdbCookie(rpmtsGetRdb(ts));
     rpm_tid_t tid = rpmtsGetTid(ts);
+
+    if (!dbus_message_append_args(msg,
+				  DBUS_TYPE_STRING, &dbcookie,
+				  DBUS_TYPE_UINT32, &tid,
+				  DBUS_TYPE_INVALID))
+	goto err;
+
+    if (!dbus_connection_send(state->bus, msg, NULL))
+	goto err;
+
+    if (snprintf(detail_name, sizeof(detail_name), "%sDetails", name) >= sizeof(detail_name)) {

> My intention here was to avoid unnecessary memory allocation, for a well-known string, controlled in the code, not passed from the outside. It's concatenating one static string with another here.

Yeah, I just didn't like the extra check & warning message I guess, but you're right, it's all just static strings defined within the plugin itself so no point in going through memory allocation.

> I'm not aware of any such (small) limitation. There are some limits (I think it's like 32KB or something for the whole message; I'd need to search for it, I'm sorry), for which one might better pass a file descriptor instead and pass the large blocks through it, but I hope it's not needed here. The dnf5 folks added such file descriptor API to the dnf5daemon last year or so. It adds many complications and is not suitable for this plugin, which only opens the D-Bus, sends some data to it, and closes itself even before the data is received to the listener.
> 
> I can limit how many packages will be provided in the list, say to 100 or 200, with a bool flag "includes_all" to indicate whether the list is cut or not. Not a great thing, but can help to avoid reaching out of the limit.

I suppose the above is regarding the package list itself. Yeah, no preference here either. That said, would it make sense to have an integer flag that controls the maximum number of packages to return?


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

Message ID: <rpm-software-management/rpm/pull/3532/review/2606579332 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20250210/3eb4a2cc/attachment.htm>