Re: Streamio and select

[yelninei--- via Bug reports for the GNU Hurd]

Sep 25, 2025, 21:02 by samuel.thiba...@gnu.org:

> Hello,
>
> yelni...@tutamail.com, le jeu. 25 sept. 2025 13:11:07 +0200, a ecrit:
>
>> Hi SamuelI found something in kmsg_putchar.
>>
>> if kmsg_write_offset +1 == kmsg_read_offset (mod KMSGBUFFSIZE) 
>> it just discards the character which leads to the previous message being 
>> malformed.
>>
>
> Which is kind of expected: we are overflowing.
>
>> Should it in that case also terminate the current message by setting the 
>> previously written char (at kmsg_write_offset -1  mod KMSGBUFFERSIZE) to \n ?
>>
>
> I'd rather not alter the content.
>
>> I tried this patch and it fixes the problem. Some messages are being lost 
>> but I don't know what else to do with them. (i am reusing the offset 
>> variable that is not needed anymore)
>> I guess I encounter this because at boot time a lot of messages are being 
>> written to the buffer in a short time before the syslogd can start emptying 
>> them so increasing the buffer prevents the early wraparound.
>>
>
> Yes, there is no way around that issue except increasing the buffer size
> if our boot log is indeed quite verbose (we can indeed do that). Perhaps
> we'd rather instead drop some messages which are not really that useful.
>

I am overflowing by just a couple of messages, with the increased buffer I can 
read 4438 bytes.
> That being said, dropping the latest characters being put might not be
> the best, I'd rather say drop the oldest characters so that you are sure
> you have the latest information. And then you'll have '\n'.
>
This essentially means writing the char as usual but when after that write and 
read offset are equal incrementing the read offset as well.

Something like this?
--8<---------------cut here---------------start------------->8---
diff --git a/device/kmsg.c b/device/kmsg.c
index e5b518e6..bb72930d 100644
--- a/device/kmsg.c
+++ b/device/kmsg.c
@@ -217,7 +217,6 @@ void
kmsg_putchar (int c)
{
   io_req_t ior;
-  int offset;
   spl_t s = -1;
 
   /* XXX: cninit is not called before cnputc is used. So call kmsginit
@@ -230,22 +229,20 @@ kmsg_putchar (int c)
 
   if (spl_init)
     s = simple_lock_irq (&kmsg_lock);
-  offset = kmsg_write_offset + 1;
-  if (offset == KMSGBUFSIZE)
-    offset = 0;
-
-  if (offset == kmsg_read_offset)
-    {
-      /* Discard C.  */
-      if (spl_init)
-       simple_unlock_irq (s, &kmsg_lock);
-      return;
-    }
 
   kmsg_buffer[kmsg_write_offset++] = c;
   if (kmsg_write_offset == KMSGBUFSIZE)
     kmsg_write_offset = 0;
 
+  if(kmsg_write_offset == kmsg_read_offset)
+    {
+      /* Drop first unread char */
+      kmsg_read_offset++;
+      if (kmsg_read_offset == KMSGBUFSIZE)
+        kmsg_read_offset = 0;
+    }
+
+
   while ((ior = (io_req_t) dequeue_head (&kmsg_read_queue)) != NULL)
     iodone (ior);
--8<---------------cut here---------------end--------------->8---

I think I would also prefer this over just dropping the character.


> Samuel
>