libdnf: fix return value of findProductId()

[ptoscano]

findProductId() returns an int, either -1 or 1 depending on whether it was possible to find the product ID in the specified certificate. The problem with this kind of return value is that's very easy to mistake for a boolean, and indeed installProductId() has a boolean-like check that it is always true, no matter the return value of findProductId().

As a fix, change the return type of findProductId() to gboolean, with true returned whether the search succeeded:

• reduce the scope of the ret_val variable, so there are less chances to use it wrongly
• make ret_val false by default, only changing it to true in the only place where it needs to be like that
• adapt the tests accordingly

Reported-by: Matyas Horky

[github-actions]

Coverage (computed on Fedora latest) •

File	Stmts	Miss	Cover	Missing
TOTAL	17896	4348	75%

report-only-changed-files is enabled. No files were changed during this commit :)

Tests	Skipped	Failures	Errors	Time
2620	6 💤	0 ❌	0 🔥	58.284s ⏱️

[ptoscano]

In case your question is: why did this "work" so far?

subscription-manager/src/plugins/libdnf/product-id.c

Lines 974 to 996 in 703c212

	int productIdFound = findProductId(pemOutput, outname);
	if (productIdFound) {
	gint ret_val = g_mkdir_with_parents(product_cert_dir, 0775);
	if (ret_val == 0) {
	gchar *productId = g_strdup(outname->str);
	int already_installed = isProductIdInstalledInDefault(productId);
	if (already_installed == 0) {
	g_string_prepend(outname, product_cert_dir);
	g_string_append(outname, ".pem");
	// TODO switch to using GFile methods to remain consistent with using GLib stuff when possible
	FILE *fileOutput = fopen(outname->str, "w+");
	if (fileOutput != NULL) {
	info("Product certificate installed to: %s", outname->str);
	fprintf(fileOutput, "%s", pemOutput->str);
	fclose(fileOutput);
	addRepoId(productDb, productId, dnf_repo_get_id(repoProductId->repo));
	ret = 1;
	} else {
	error("Unable write to file with certificate file :%s", outname->str);
	}
	}
	g_free(productId);

• findProductId() returned -1, so the if (productIdFound) was executed; because of the failure, outname was still set as empty string
• isProductIdInstalledInDefault() returned 0 (since no installed product ID is ""
• because of the empty outname, the result of g_string_prepend/g_string_append was /tmp.pem (product_cert_dir == /tmp)
• when running as non-root, it is not possible to write to /tmp.pem, and thus the fopen() call failed, and "Unable write to file with certificate file was logged as error
• when running as root, it is possible to write to /tmp.pem, repoProductId->repo was not initialized, leading to dnf_repo_get_id() crashing

[jirihnidek]

LGTM