[Rpm-maint] [rpm-software-management/rpm] Major cleanup of the rpmInstall() structures (PR #4073)

[Michal Domonkos]

@dmnks requested changes on this pull request.

This one got a bit delayed in the review queue, sorry about that 😄

Looks nice indeed, and makes the code sooo much easier to follow, and (hopefully) safer too 👍 

> +
+    switch (urlIsURL(fileURL)) {
+    case URL_IS_HTTPS:
+    case URL_IS_HTTP:
+    case URL_IS_FTP:
+	if (rpmIsVerbose())
+	    fprintf(stdout, _("Retrieving %s\n"), fileURL);
+
+	tfd = rpmMkTempFile(rpmtsRootDir(ts), &tfn);
+	if (tfn) {
+	    Fclose(tfd);
+	    rc = urlGetFile(fileURL, tfn);
+	    fn = tfn;
+	}
+	if (rc)
+	    rpmlog(RPMLOG_ERR, _("skipping %s - transfer failed\n"), fileURL);

We should probably set `fn` to the empty string here, otherwise we would return an actual filename if `urlGetFile()` fails (see line 428) and thus indicate success to the caller.

> +	if (rc || ac == 0) {
+	    if (giFlags & RPMGI_NOGLOB) {
+		rpmlog(RPMLOG_ERR, _("File not found: %s\n"), *fnp);
+	    } else {
+		rpmlog(RPMLOG_ERR, _("File not found by glob: %s\n"), *fnp);
+	    }
+	    numFailed++;
+	} else {
+	    queue.insert(queue.end(), av, av+ac);
+	}
+	argvFree(av);
+    }
+    return numFailed;
+}
+
+std::string getFile(rpmts ts, const std::string & arg)

Shouldn't this be `static`?

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

Message ID: <rpm-software-management/rpm/pull/4073/review/3745918280 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20260203/10ffb9c6/attachment.htm>