[LEDE-DEV] [PATCH] ubox logread

[Felix Fietkau]

On 2017-11-06 17:45, Maksym Ruchko wrote:
>> I think your patch was only masking the real issues.
> Just to explain what was my rationale - I was seeing that the old logread code was blocked when syslog message was occasionally crossing the 4K boundary, 
> then in the logread.c function logread_fd_data_cb  condition if (len < cur_len) was always true if boundary crossing happened.
> I think the ustream code by default allocates single 4K reader buffer and unlimited number of write buffers and reader uses one buffer at most anyway.
> My patch  allowed  unlimited buffers on the reader side and made sure blob is read to the end, possibly not in the most elegant way.
> 
>>I reviewed the logd and logread code and found quite a few things to be broken.
>>I have now reworked the code quite a bit, and your test case also works nicely with the updated code, please test the latest version.
> I cannot reproduce the problem with your patches applied.
> But I am failing to understand how exactly the problem is solved. I have a feeling that the limiting the message size in the logd.c was the cure.
> This:
> @@ -149,10 +167,18 @@
>         char *event;
>  
>         if (msg) {
> +               int len;
> +
>                 blobmsg_parse(&write_policy, 1, &tb, blob_data(msg), blob_len(msg));
>                 if (tb) {
>                         event = blobmsg_get_string(tb);
> -                       log_add(event, strlen(event) + 1, SOURCE_SYSLOG);
> +                       len = strlen(event) + 1;
> +                       if (len > LOG_LINE_SIZE) {
> +                               len = LOG_LINE_SIZE;
> +                               event[len - 1] = 0;
> +                       }
> +
> +                       log_add(event, len, SOURCE_SYSLOG);
>                 }
>         }
> 
> But then boundary crossing on 4K is still theoretically possible, just much less likely?
It's not possible anymore, because ustream itself takes care of that
corner case in ustream_prepare_buf. When it tries to do another read and
there is not enough room because of existing data at the end of the
buffer, it moves the data to the beginning of the buffer.

This wasn't working before, because the code was calling uloop_end after
the first data callback run.

I've fixed a lot of issues here, not just a simple buffer management
problem.

- Felix