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

[Michal Domonkos]

@dmnks commented on this pull request.

Please split the addition of the xml file into a separate commit. Some additional comments inline.

> +    array = (char **) malloc((nElems + 1 ) * sizeof(char *));
+    if (array == NULL)
+        goto err;
+
+    array[0] = NULL;
+    for (i = 0; i < nElems; i++) {
+        rpmte te = rpmtsElement(ts, i);
+        char *buff;
+        int buffln;
+        const char *op = "??";
+        const char *nevra = rpmteNEVRA(te);
+        if (nevra == NULL)
+            nevra = "";
+        switch (rpmteType (te)) {
+        case TR_ADDED:
+            op = "added";

Not a huge deal but I wonder if the `op` names shouldn't be the same as we return from `rpmteTypeString()` (internal function), just for consistency.

Ideally, we would just use that function here but alas, it returns *translated* strings.

> +        case TR_REMOVED:
+            op = "removed";
+            break;
+        case TR_RPMDB:
+            op = "rpmdb";
+            break;
+        case TR_RESTORED:
+            op = "restored";
+            break;
+        }
+        buffln = strlen(op) + 1 + strlen(nevra) + 1;
+        buff = (char *) malloc(sizeof(char) * buffln);
+        if (buff == NULL)
+            goto err;
+	/* encode as "operation SPACE nevra" */
+	snprintf(buff, buffln, "%s %s", op, nevra);

Maybe using rpm's own string functions (i.e. `rstrcat()` in this  case) would be better here (although again, doesn't matter too much).

> @@ -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)) {

Just curious - is there some kind of limitation on the string size being passed to the dbus API? Can't we just pass a dynamically allocated string here (and free it afterwards)?

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

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