feat: use nvme_show_err() helper function

[ikegami-t]

It introduced to handle the negative error and the postive status codes. Then reduce the duplicated error and status codes output implementation.

[ikegami-t]

This was mentioned by the PR MR comment: #3132 (comment).

[igaw]

I'd suggest to just use nvme_show_err. The chance that vasprintf fails is rather slim, so I don't think it justifies this additional complexity.

[ikegami-t]

Only the char pointer msg can be passed for nvme_show_err as below.

void nvme_show_err(const char *msg, int err)

But show_err added can be passed the char pointer fmt and the variable length arguments ... also as below.

static void show_err(int err, const char *func, const char *fmt, ...)

Since the caller functions in feat-nvme.c pass the variable: power_mgmt_feat, etc. also as below.

- 		nvme_show_perror("Set %s", power_mgmt_feat);
+ 	if (err) {
+ 		show_error(err, "Set %s", power_mgmt_feat);

By the way is it okay if deleted the paramater func and use the alloc_error string instead?

static void show_error(int err, const char *fmt, ...)
{
	va_list ap;

	_cleanup_free_ char *msg = NULL;

	va_start(ap, fmt);

	if (vasprintf(&msg, fmt, ap) < 0)
		msg = NULL;

	nvme_show_err(msg ? msg : alloc_error, err);

	va_end(ap);
}

Still we should use nvme_show_err directly? If so I will do use it and just pass the char pointer msg by the caller functions.

[igaw]

We have a bunch error reporting helpers in nvme.c:

nvme_show_error, nvme_show_status and nvme_show_error_status.

I'd rather have them made better and usable for the whole code base instead introducing per file new versions of printing stuff. The big plan is to get rid of plain printf and use function from nvme-print.h which is the output format aware.

[ikegami-t]

Understood so will use nvme_show_err as suggested and also thinking to change nvme_show_err itself to pass the char pointer fmt and the variable length arguments. Thank you.

[igaw]

If there is an error just return. This follows the same pattern as we have in nvme.c now.

err = nvme_get_features(...);
if (err)
   return;

nvme_feature_show(...);

[ikegami-t]

Yes will do that. nvme_show_init needed to call nvme_show_finish as a set without return so change the order as below.

	err = nvme_get_features(hdl, 0, fid, sel, cdw11, uidx, buf, len,
				&result);
	if (err) {
		show_error(err, "Get %s", feat);
		return err;
	}

	nvme_show_init();

	nvme_feature_show(fid, sel, result);
	if (NVME_CHECK(sel, GET_FEATURES_SEL, SUPPORTED))
		nvme_show_select_result(fid, result);
	else
		nvme_feature_show_fields(fid, result, buf);

	nvme_show_finish();

[ikegami-t]

Just pushed the commit fixed for the comments but still using show_error but not using nvme_show_err directly so if needed will do update the changes again to use nvme_show_err directly.

[ikegami-t]

Just repushed the paches as including the changes for the nvme_show_err parameters order.