ClosedPublic
Actions

Authored by dumbbell on Jan 4 2026, 10:18 AM.

Details

Reviewers

Group Reviewers

linuxkpi

Commits

rG710167623d82: linuxkpi: Implement <linux/seq_buf.h>

Summary

It is a wrapper above a char * to track the overall available space in the buffer as well as the used space. This wrapper does not manage memory allocation.

The DRM generic code started to use this in Linux 6.10.

This is part of the update of DRM drivers to Linux 6.10.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

dumbbell created this revision.Jan 4 2026, 10:18 AM

Herald added subscribers: emaste, imp. · View Herald TranscriptJan 4 2026, 10:18 AM

dumbbell requested review of this revision.Jan 4 2026, 10:18 AM

Use SPDX tag and remove license text.

bz requested changes to this revision.Jan 6 2026, 12:48 AM

bz added a subscriber: bz.

bz added inline comments.

sys/compat/linuxkpi/common/include/linux/seq_buf.h
2	Not needed anymore.
3	Could move this down in this case given it's not committed yet.
9	use SPDX?
37	While this is probably sufficient, does that mean we can leave actual data behind in the buffer? Should we zero the entire buffer? I do not know how this is used, hence asking as there is a cost to zeroing...
55	I would prefer if the public KPI functions would be named linuxkpi_ and "internal" functions get the "lkpi_" prefix. I think during the last year we slowly got to that and it would help if we try to converge to that for everything new.
sys/compat/linuxkpi/common/src/linux_seq_buf.c
41	You probably want to print to buffer + len and only do that if len < size (not overflowed yet) and then the s->size needs to be reduced by len as well?
43	An overflow of the buffer is also considered an error so we'll need to check if len + ret < size and only if so update len and return success. This likely needs to be += then.
45	According to documentation this returns 0 on success and -1 on error, not the length it would have printed.

This revision now requires changes to proceed.Jan 6 2026, 12:48 AM

christos added a subscriber: christos.Jan 6 2026, 11:13 AM

christos added inline comments.

sys/compat/linuxkpi/common/include/linux/seq_buf.h
37	I asked the same in an email a while ago. Isn't it risky to simply prepend a \0 at the beginning and leave the previous data still there, instead of zeroing out the whole buffer?

emaste added inline comments.Jan 22 2026, 4:34 PM

sys/compat/linuxkpi/common/include/linux/seq_buf.h
37	Linux only puts a `\0` in the first byte so assuming we don't use this in any kernel-user contexts beyond what Linux does, and assuming an absence of Linux bugs, it's OK. I see use in 3 files in drm: drivers/gpu/drm/drm_edid.c drivers/gpu/drm/i915/display/intel_ddi.c drivers/gpu/drm/i915/display/intel_dp.c Used only to craft strings followed by `seq_buf_str()` to get a `char *` to pass to e.g. printf. So OK. (By comparison `sbuf_clear` leaves the buffer intact, it does not even put a `\0` in the first byte.)