This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Add a close-on-exec check on accept() in Android module.
ClosedPublic

Authored by chh on Jul 13 2017, 10:29 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

yawanng created this revision.Jul 13 2017, 10:29 AM
yawanng retitled this revision from [clang-tidy] Add close-on-exec checks on several functions in Android module. to [clang-tidy] Add a close-on-exec check on accept() in Android module..Jul 13 2017, 10:56 AM
yawanng edited the summary of this revision. (Show Details)
yawanng edited the summary of this revision. (Show Details)Jul 13 2017, 11:07 AM
Eugene.Zelenko edited reviewers, added: alexfh, hokein, aaron.ballman; removed: chh.Jul 13 2017, 11:16 AM
Eugene.Zelenko set the repository for this revision to rL LLVM.
chh added a subscriber: chh.Jul 13 2017, 11:58 AM
alexfh requested changes to this revision.Jul 15 2017, 1:31 AM

See the comment on D35372.

This revision now requires changes to proceed.Jul 15 2017, 1:31 AM
yawanng updated this revision to Diff 110659.Aug 10 2017, 5:16 PM
yawanng edited edge metadata.
yawanng updated this revision to Diff 110758.Aug 11 2017, 10:14 AM
alexfh accepted this revision.Aug 16 2017, 8:27 AM

LG with a nit.

clang-tidy/android/CloexecAcceptCheck.cpp
42 ↗(On Diff #110758)

Two minor issues here:

  1. FixMsg name is misleading, specifically the "Msg" part, since it's not a message.
  2. there's no need to use an argument comment, where it's clear what the actual argument's meaning is. E.g. ReplacementText is pretty clear on its own. As is the warning message above.
This revision is now accepted and ready to land.Aug 16 2017, 8:27 AM
chh commandeered this revision.Aug 16 2017, 10:14 AM
chh added a reviewer: yawanng.
chh added inline comments.
clang-tidy/android/CloexecAcceptCheck.cpp
42 ↗(On Diff #110758)

Done.

chh updated this revision to Diff 111377.Aug 16 2017, 10:15 AM
This revision was automatically updated to reflect the committed changes.