ping: Don't mix stdio and direct writes
ClosedPublic
Actions

Authored by des on Fri, Jun 26, 2:03 PM.

Details

Reviewers

markj
siva

Commits

rG8bda488114f3: ping: Don't mix stdio and direct writes

Summary

POSIX does not allow mixing direct writes to STDOUT_FILENO with stdio
operations on stdout like we do here. More importantly, it causes
tests to fail randomly (or not-so-randomly after I added an fflush in
a previous commit).

MFC after: 1 week

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

des created this revision.Fri, Jun 26, 2:03 PM

Herald added a subscriber: imp. · View Herald TranscriptFri, Jun 26, 2:03 PM

des requested review of this revision.Fri, Jun 26, 2:03 PM

Harbormaster completed remote builds in B74300: Diff 180712.Fri, Jun 26, 2:03 PM

@siva this change is required for your PR to work reliably

markj added inline comments.Fri, Jun 26, 2:05 PM

sbin/ping/ping.c
986	Why not use putchar() throughout?

des added inline comments.Fri, Jun 26, 2:09 PM

sbin/ping/ping.c
986	I don't like mixing explicit and implicit interfaces. If you're worried about performance, I suppose I can use `putc()` instead of `fputc()`.

putc

Harbormaster completed remote builds in B74301: Diff 180713.Fri, Jun 26, 2:11 PM

des marked an inline comment as done.Fri, Jun 26, 2:11 PM

markj accepted this revision.Fri, Jun 26, 2:22 PM

markj added inline comments.

sbin/ping/ping.c
986	Okay, ping is still using printf() to print its messages though. As far as I can tell, putc() and fputc() are identical...

This revision is now accepted and ready to land.Fri, Jun 26, 2:22 PM

des added inline comments.Fri, Jun 26, 2:45 PM

sbin/ping/ping.c
986	`putc()` is a macro which, if unthreaded (as is the case here) calls `__sputc()` directly without locking the stream, while `fputc()` is a function that first locks the stream before calling `__sputc()`.