Paths

Table of Contentst

Differential D19188

Make `server_cat(..)` handle short receives
ClosedPublic
Actions

Authored by ngie on Feb 14 2019, 1:04 AM.

Details

Reviewers

asomers
emaste
jtl
lwhsu

Commits

rS344310: Make `server_cat(..)` handle short receives

Summary

In short, the prior code was far too simplistic when it came to calling recv(2)
and failed intermittently (or in the case of Jenkins, deterministically).

Handle short recv(2)s by checking the return code and incrementing the window
into the buffer by the number of received bytes. If the number of received
bytes <= 0, then bail out and test the total number of received bytes vs the
expected number of bytes sent for equality and base whether or not the test
passes/fails on that fact.

Remove the expected failure, now that the testcase deterministically
passes [1].

PR: 234809 [1], 235200
MFC after: 1 week

Test Plan

kyua test passes all testcases, sans the ones expected to fail in bug 232210.
make tinderbox passed.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ngie created this revision.Feb 14 2019, 1:04 AM

Herald added a subscriber: imp. · View Herald TranscriptFeb 14 2019, 1:04 AM

Harbormaster completed remote builds in B22492: Diff 53901.Feb 14 2019, 1:04 AM

emaste added inline comments.Feb 14 2019, 1:29 AM

lib/libc/tests/sys/sendfile_test.c
159 ↗	(On Diff #53901)	Was this needed, or just defensive?
210 ↗	(On Diff #53901)	`#define` seems strange here when it's just used once in the calculation of buffer_size below

ngie added inline comments.Feb 14 2019, 1:32 AM

lib/libc/tests/sys/sendfile_test.c
159 ↗	(On Diff #53901)	I needed to add this, otherwise it would doubleprint the messages. It's an unfortunate side-effect from using ATF I think :/...
210 ↗	(On Diff #53901)	I wanted to provide a `#define` so the constant was self-documenting, but I could move it down to the initialization, along with the comment though.

Address @emaste's comments

Harbormaster completed remote builds in B22552: Diff 54034.Feb 18 2019, 9:31 AM

@asomers, @emaste, @jtl, @lwhsu: ping?

ngie edited the test plan for this revision. (Show Details)Feb 18 2019, 11:43 AM

LGTM!

lib/libc/tests/sys/sendfile_test.c
220 ↗	(On Diff #54034)	So the intent of the `+ 10` is to ensure that when `recv_ret == 0`, it's because everything sent was received rather than we simply ran out of space in the receive buffer?

This revision is now accepted and ready to land.Feb 19 2019, 7:23 PM

ngie marked an inline comment as done.Feb 19 2019, 7:26 PM

ngie added inline comments.

lib/libc/tests/sys/sendfile_test.c
220 ↗	(On Diff #54034)	That's absolutely correct :)! I wanted to make sure that the interface functioned as designed and not purely by accident by adding in additional buffer to capture accidental "over sends". If I set the buffer to the exact size of the expected message, I couldn't test for that.

ngie marked an inline comment as done.Feb 19 2019, 7:27 PM

Approved

Closed by commit rS344310: Make `server_cat(..)` handle short receives (authored by ngie). · Explain WhyFeb 19 2019, 10:19 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

head/

lib/

libc/

tests/

sys/

sendfile_test.c

54 lines

Diff 54096

View Options

head/lib/libc/tests/sys/sendfile_test.c

Make `server_cat(..)` handle short receivesClosedPublicActions