Linas-san,

On Thu, Dec 14, 2006 at 10:42:16AM +0900, Ishizaki Kou wrote:

quoted

Linas-san,

Thanks for your comment.

quoted

On Tue, Dec 12, 2006 at 12:31:29PM +0900, Ishizaki Kou wrote:

quoted

+
+static int hvc_beat_get_chars(uint32_t vtermno, char *buf, int cnt)
+{
+	unsigned long kb[2];
+	unsigned long got;
+
+	if (beat_get_term_char(vtermno, &got, &kb[0], &kb[1]) == 0) {
+	   memcpy(buf, kb, got);
+		              return got;

quoted

This seems to completely ignore "cnt". Thus, I presume that
beat_get_term_char might return more chars than there is room for in buf,
thus corrupting something, somewhere.

quoted

This depends "beat_get_term_char" returns only one character at once
(for now), and assumes cnt > 0. This assumption will reduce code for
now.

But it will break in the future. If new firmware is released for beat,
that returns more than one char, then it will silently corrupt old kernels
on rare occasions, making the problem hard to find. What's more, the 
firmware people might forget to tell you about thier change, and
so you won't submit a matching update to the linux kernel. (Or you may
have a new job by then, or have lost interest/got bored, etc.)
Years later, someone will finally debug the problem, after a long and
difficult search, and they'll curse this code.  Better do it right, now.

Okay, we will do.

We have to remark 'this is NOT FOR PRODUCT,' since console I/O of Beat
is HORRIBLY slow (putting characters to console waits till
the characters actually putted to the serial port. No queue is available.)

quoted

quoted

quoted

+static int hvc_beat_put_chars(uint32_t vtermno, const char *buf, int cnt)
+{
+	unsigned long kb[2];
+
+	memcpy(kb, buf, sizeof(kb));
+	beat_put_term_char(vtermno, cnt, kb[0], kb[1]);
+	return cnt;
+}

quoted

I can't imagine how this can possibly work. 
What if "cnt" is greater than 8?

This routine assumes that 0 <= cnt <= 16, that is already checked by
caller. (Note that "unsigned long" is 8 bytes long at ppc64)

Actually, its N_OUTBUF which is currently defined to be 16. However,
that may someday change, in which case you'd have a bug, again.

Okay, we understand your opinion, and will do so.

Best regards,
Kou Ishizaki.