Page MenuHomeFreeBSD

FFL: Change type of tcp_output() len variable
AbandonedPublic

Authored by jtl on Jul 2 2016, 8:28 PM.

Details

Reviewers
jtl
lstewart
bz
Group Reviewers
transport
Summary

In tcp_output, change len from platform-dependent long to fixed-length int32_t.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 4443
Build 4494: arc lint + arc unit

Event Timeline

jtl updated this revision to Diff 18096.Jul 2 2016, 8:28 PM
jtl retitled this revision from to FFL: Change type of tcp_output() len variable.
jtl updated this object.
jtl edited the test plan for this revision. (Show Details)
jtl added a reviewer: transport.
Herald added a subscriber: imp. · View Herald Transcript
bz requested changes to this revision.Jul 3 2016, 12:27 AM
bz added a reviewer: bz.
bz added a subscriber: bz.
bz added inline comments.
sys/netinet/tcp_output.c
1307

Aehm, no; change the format specifier, don't cast!

This revision now requires changes to proceed.Jul 3 2016, 12:27 AM
jtl added inline comments.Jul 3 2016, 12:38 AM
sys/netinet/tcp_output.c
1307

You're going to see this in several of my reviews. The "right thing" (TM) to do is to use the PRId32 macro. But, I didn't want to do that, since we don't use it anywhere else in the kernel code.

But, thinking through this more, an int format specifier should work for this. We don't have platforms with ints less than 32 bits, and aren't likely to get one in the future. I'll get all those reviews updated and resubmit.

jtl updated this revision to Diff 18191.Jul 6 2016, 2:44 PM
jtl edited edge metadata.

Remove cast and change format string in KASSERT.

jtl marked 2 inline comments as done.Jul 6 2016, 2:46 PM
lstewart accepted this revision.Aug 24 2016, 3:15 AM
lstewart added a reviewer: lstewart.
lstewart added a subscriber: lstewart.
lstewart added inline comments.
sys/netinet/tcp_output.c
281

The think I agree with your offline comment about consistency - either make this signed, or change elsewhere to unsigned.

303

Why not min() here and lines 306, 379?

596

(uint32_t)(len + off) here and elsewhere below perhaps?

jtl accepted this revision.Oct 6 2016, 7:09 PM
jtl abandoned this revision.
jtl added a reviewer: jtl.

Committed in rS306769.