Re: [kvm-unit-tests PATCH v2 5/5] s390x: SCLP unit test

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/11/2019 12.19, Claudio Imbrenda wrote:
On Mon, 4 Nov 2019 10:45:07 +0100
Janosch Frank <frankja@xxxxxxxxxxxxx> wrote:
[...]
+static void test_toolong(void)
+{
+	uint32_t cmd = SCLP_CMD_WRITE_EVENT_DATA;
+	uint16_t res = SCLP_RC_SCCB_BOUNDARY_VIOLATION;

Why use variables for constants that are never touched?

readability mostly. the names of the constants are rather long.
the compiler will notice it and do the Right Thing™

I'd like to suggest to add the "const" keyword to both variables in that case, then it's clear that they are not used to be modified.

+		h->length = 4096;
+
+		valid_code = commands[i];
+		cc = sclp_service_call(commands[i], h);
+		if (cc)
+			break;
+		if (h->response_code ==
SCLP_RC_NORMAL_READ_COMPLETION)
+			return;
+		if (h->response_code !=
SCLP_RC_INVALID_SCLP_COMMAND)
+			break;

Depending on line length you could add that to the cc check.
Maybe you could also group the error conditions before the success
conditions or the other way around.

yeah it woud fit, but I'm not sure it would be more readable:

if (cc || (h->response_code != SCLP_RC_INVALID_SCLP_COMMAND))
                         break;

In case you go with that solution, please drop the innermost parentheses.

 Thomas





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux