Avoid conditional references of PRINTF_BUFR_SIZE in headers.
ClosedPublic
Actions

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

Details

Reviewers

cem
ed
dim

Commits

rS340208: Avoid fixing the tty_info() buffer size in tty.h.

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 Passed

Unit

No Test Coverage

Build Status

Buildable 20670
Build 20085: arc lint + arc unit

Event Timeline

markj created this revision.Nov 6 2018, 9:48 PM

Harbormaster completed remote builds in B20668: Diff 50086.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–137	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–137	Hrm, ok. I'll restore the conditional definition then.
137	I guess we might as well use C99. I'll change it.

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

Harbormaster completed remote builds in B20670: Diff 50094.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

Closed by commit rS340208: Avoid fixing the tty_info() buffer size in tty.h. (authored by markj). · Explain Why

This revision was automatically updated to reflect the committed changes.

Herald added a subscriber: imp. · View Herald TranscriptNov 6 2018, 11:41 PM

(Approved)

ed added inline comments.Nov 7 2018, 7:02 AM