Re: [kvm-unit-tests PATCH v3 1/4] lib: s390x: add support for SCLP console read

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

 



On 4/22/22 09:50, Nico Boehr wrote:
On Thu, 2022-04-21 at 16:29 +0200, Janosch Frank wrote:

[...]
diff --git a/lib/s390x/sclp-console.c b/lib/s390x/sclp-console.c
index fa36a6a42381..8c4bf68cbbab 100644
--- a/lib/s390x/sclp-console.c
+++ b/lib/s390x/sclp-console.c
[...]
+       read_buf_end = sccb->ebh.length -
event_buffer_ascii_recv_header_len;

Isn't this more like a length of the current read buffer contents?

Right, thanks, length is a much better name.

[...]
diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
index fead007a6037..e48a5a3df20b 100644
--- a/lib/s390x/sclp.h
+++ b/lib/s390x/sclp.h
@@ -313,6 +313,14 @@ typedef struct ReadEventData {
         uint32_t mask;
   } __attribute__((packed)) ReadEventData;
+#define SCLP_EVENT_ASCII_TYPE_DATA_STREAM_FOLLOWS 0

Hrm, I'm not completely happy with the naming here since I confused
it
to the ebh->type when looking up the constants. But now I understand
why
you chose it.

Yeah, it sure is confusing.

Maybe it is better if we leave out the "type" entirely, but this might
make it harder to understand where it's coming from:
SCLP_ASCII_RECEIVE_DATA_STREAM_FOLLOWS

Another alternative I thought about is using enums, it won't fix the
naming, but at least it might be clearer to which type it belongs.

Let me know what you think.

It should be fine as is. I don't expect that we have to touch this file very often.




[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