Page MenuHomeFreeBSD

Avoid conditional references of PRINTF_BUFR_SIZE in headers.
ClosedPublic

Authored by markj on Nov 6 2018, 9:48 PM.

Details

Summary

It turns out that PRINTF_BUFR_SIZE is always defined in opt_printf.h, but
most C files don't include that. However, subr_prf.c does, and it also
references struct tty. PRINTF_BUFR_SIZE defaults to 128, so the size of
t_prbuf varies depending on the compilation unit, resulting in duplicate
CTF type definitions.

Fix the problem by always using PRINTF_BUFR_SIZE to size t_prbuf.

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 20668
Build 20083: arc lint + arc unit

Event Timeline

markj created this revision.Nov 6 2018, 9:48 PM
markj added reviewers: cem, ed, dim.Nov 6 2018, 9:49 PM
cem accepted this revision.Nov 6 2018, 10:32 PM
cem added inline comments.
sys/sys/tty.h
136

I think some universe configs don't define PRINTF_BUFR_SIZE.

137

Is [0] or [] the preferred style? Either way, it's ok.

This revision is now accepted and ready to land.Nov 6 2018, 10:32 PM
markj marked 2 inline comments as done.Nov 6 2018, 10:45 PM
markj added inline comments.
sys/sys/tty.h
136

Hrm, ok. I'll restore the conditional definition then.

137

I guess we might as well use C99. I'll change it.

markj updated this revision to Diff 50094.Nov 6 2018, 11:07 PM
markj marked 2 inline comments as done.
  • Handle PRINTF_BUFR_SIZE not being defined.
  • Ensure that the buffer size is consistent across all CUs. That is, don't rely on CUs to remember to include opt_printf.h.
This revision now requires review to proceed.Nov 6 2018, 11:07 PM
This revision was not accepted when it landed; it landed in state Needs Review.Nov 6 2018, 11:41 PM
This revision was automatically updated to reflect the committed changes.
cem added a comment.Nov 7 2018, 12:27 AM

(Approved)

ed added inline comments.Nov 7 2018, 7:02 AM
head/sys/kern/tty.c
110 ↗(On Diff #50096)

If PRINTF_BUFR_SIZE is always defined in opt_printf.h, why do we need the conditional code here?

cem added inline comments.Nov 7 2018, 7:15 AM
head/sys/kern/tty.c
110 ↗(On Diff #50096)

It's not always defined, IIRC. Hence similar logic in cam_xpt.c, subr_prf.c, and xen_console.c.

markj marked an inline comment as done.Nov 7 2018, 4:05 PM
markj added inline comments.
head/sys/kern/tty.c
110 ↗(On Diff #50096)

Indeed, there is no fallback definition that gets used if it's omitted in the kernel configuration file.