This is an archive of the discontinued LLVM Phabricator instance.

[Tsan] Fix the getline_nohang.cc test to build on FreeBSD
ClosedPublic

Authored by kutuzov.viktor.84 on Oct 8 2014, 4:58 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

kutuzov.viktor.84 retitled this revision from to [Tsan] Fix the getline_nohang.cc test to build on FreeBSD.
kutuzov.viktor.84 updated this object.
kutuzov.viktor.84 edited the test plan for this revision. (Show Details)
kutuzov.viktor.84 added reviewers: kcc, samsonov, glider.
kutuzov.viktor.84 added subscribers: Unknown Object (MLST), emaste.
glider added inline comments.Oct 8 2014, 5:13 AM
test/tsan/getline_nohang.cc
5 ↗(On Diff #14560)

Isn't "#ifdef FreeBSD" more common? (that doesn't make much difference however)

I've also noticed other #ifdef's for OpenBSD, DragonFlyBSD, NetBSD etc. in the codebase. Do you know if they also apply here?

emaste added inline comments.Oct 8 2014, 8:12 AM
test/tsan/getline_nohang.cc
5 ↗(On Diff #14560)

Isn't "#ifdef FreeBSD" more common?

That's the style I would use, fwiw.

I've also noticed other #ifdef's for OpenBSD, DragonFlyBSD, NetBSD etc. in the codebase. Do you know if they also apply here?

There's going to be a lot of tedious comparison work to be done to make this work across all *BSDs.

On the LLDB list we had a short discussion on having a general *BSD define, but did not reach a conclusion. My take is that there's not a lot of value in adding other BSD defines for a small handful of cases (like this one, perhaps) if the overall functionality is nowhere near complete.

Isn't "#ifdef FreeBSD" more common? (that doesn't make much difference however)

Yes, a typo. Thanks.

I've also noticed other #ifdef's for OpenBSD, DragonFlyBSD, NetBSD etc. in the codebase. Do you know if they also apply here?

Well, I don't. But even if I knew it, I would propose we do not respect them in this diff.

#if replaced with #ifdef.

glider accepted this revision.Oct 9 2014, 3:17 AM
glider edited edge metadata.

LGTM

This revision is now accepted and ready to land.Oct 9 2014, 3:17 AM
Diffusion closed this revision.Oct 10 2014, 12:18 AM
Diffusion updated this revision to Diff 14704.

Closed by commit rL219482 (authored by vkutuzov).