Skip to content

Commit bdb3ffe

Browse files
cataphractcataphract
committed
Use owned_fd
1 parent 8659010 commit bdb3ffe

5 files changed

Lines changed: 71 additions & 65 deletions

File tree

‎appsec/.clang-tidy‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
---
22
# readability-function-cognitive-complexity temporarily disabled until clang-tidy is fixed
33
# right now emalloc causes it to misbehave
4-
Checks: '*,-bugprone-reserved-identifier,-hicpp-signed-bitwise,-llvmlibc-restrict-system-libc-headers,-altera-unroll-loops,-hicpp-named-parameter,-cert-dcl37-c,-cert-dcl51-cpp,-read,-cppcoreguidelines-init-variables,-cppcoreguidelines-avoid-non-const-global-variables,-altera-id-dependent-backward-branch,-performance-no-int-to-ptr,-altera-struct-pack-align,-google-readability-casting,-modernize-use-trailing-return-type,-llvmlibc-implementation-in-namespace,-llvmlibc-callee-namespace,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-fuchsia-default-arguments-declarations,-fuchsia-overloaded-operator,-cppcoreguidelines-pro-type-union-access,-fuchsia-default-arguments-calls,-cppcoreguidelines-non-private-member-variables-in-classes,-misc-non-private-member-variables-in-classes,-google-readability-todo,-llvm-header-guard,-readability-function-cognitive-complexity,-readability-identifier-length,-modernize-macro-to-enum,-misc-include-cleaner,-bugprone-empty-catch,-cppcoreguidelines-avoid-do-while,-hicpp-no-array-decay'
4+
Checks: '*,-bugprone-reserved-identifier,-hicpp-signed-bitwise,-llvmlibc-restrict-system-libc-headers,-altera-unroll-loops,-hicpp-named-parameter,-cert-dcl37-c,-cert-dcl51-cpp,-read,-cppcoreguidelines-init-variables,-cppcoreguidelines-avoid-non-const-global-variables,-altera-id-dependent-backward-branch,-performance-no-int-to-ptr,-altera-struct-pack-align,-google-readability-casting,-modernize-use-trailing-return-type,-llvmlibc-implementation-in-namespace,-llvmlibc-callee-namespace,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-fuchsia-default-arguments-declarations,-fuchsia-overloaded-operator,-cppcoreguidelines-pro-type-union-access,-fuchsia-default-arguments-calls,-cppcoreguidelines-non-private-member-variables-in-classes,-misc-non-private-member-variables-in-classes,-google-readability-todo,-llvm-header-guard,-readability-function-cognitive-complexity,-readability-identifier-length,-modernize-macro-to-enum,-misc-include-cleaner,-bugprone-empty-catch,-cppcoreguidelines-avoid-do-while,-hicpp-no-array-decay,-llvmlibc-*'
55
WarningsAsErrors: '*'
66
HeaderFilterRegex: ''
77
CheckOptions:

‎appsec/src/helper/network/acceptor.cpp‎

Lines changed: 39 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -23,59 +23,55 @@ namespace dds::network::local {
2323
acceptor::acceptor(const std::string_view &sv)
2424
{
2525
// NOLINTNEXTLINE(android-cloexec-socket,cppcoreguidelines-prefer-member-initializer)
26-
sock_ = ::socket(AF_UNIX, SOCK_STREAM, 0);
27-
if (sock_ == -1) {
26+
sock_ = owned_fd{::socket(AF_UNIX, SOCK_STREAM, 0)};
27+
if (sock_.is_empty()) {
2828
throw std::system_error(errno, std::generic_category());
2929
}
3030

31-
try {
32-
struct sockaddr_un addr {};
33-
addr.sun_family = AF_UNIX;
34-
if (sv.size() > sizeof(addr.sun_path) - 1) {
35-
throw std::invalid_argument{"socket path too long"};
36-
}
37-
strcpy(static_cast<char *>(addr.sun_path), sv.data()); // NOLINT
31+
struct sockaddr_un addr {};
32+
addr.sun_family = AF_UNIX;
33+
if (sv.size() > sizeof(addr.sun_path) - 1) {
34+
throw std::invalid_argument{"socket path too long"};
35+
}
36+
strcpy(static_cast<char *>(addr.sun_path), sv.data()); // NOLINT
3837

39-
// Remove the existing socket
40-
int res = ::unlink(static_cast<char *>(addr.sun_path));
41-
if (res == -1 && errno != ENOENT) {
42-
SPDLOG_ERROR("Failed to unlink {}: errno {}", addr.sun_path, errno);
43-
throw std::system_error(errno, std::generic_category());
44-
}
45-
SPDLOG_DEBUG("Unlinked {}", addr.sun_path);
38+
// Remove the existing socket
39+
int res = ::unlink(static_cast<char *>(addr.sun_path));
40+
if (res == -1 && errno != ENOENT) {
41+
SPDLOG_ERROR("Failed to unlink {}: errno {}", addr.sun_path, errno);
42+
throw std::system_error(errno, std::generic_category());
43+
}
44+
SPDLOG_DEBUG("Unlinked {}", addr.sun_path);
4645

47-
res =
48-
// NOLINTNEXTLINE
49-
::bind(sock_, reinterpret_cast<struct sockaddr *>(&addr),
50-
sizeof(addr));
51-
if (res == -1) {
52-
SPDLOG_ERROR(
53-
"Failed to bind socket to {}: errno {}", addr.sun_path, errno);
54-
throw std::system_error(errno, std::generic_category());
55-
}
46+
res =
47+
// NOLINTNEXTLINE
48+
::bind(sock_.get(), reinterpret_cast<struct sockaddr *>(&addr),
49+
sizeof(addr));
50+
if (res == -1) {
51+
SPDLOG_ERROR(
52+
"Failed to bind socket to {}: errno {}", addr.sun_path, errno);
53+
throw std::system_error(errno, std::generic_category());
54+
}
5655

57-
res = ::chmod(sv.data(), 0777); // NOLINT
58-
if (res == -1) {
59-
SPDLOG_ERROR(
60-
"Failed to chmod socket {}: errno {}", addr.sun_path, errno);
61-
throw std::system_error(errno, std::generic_category());
62-
}
56+
res = ::chmod(sv.data(), 0777); // NOLINT
57+
if (res == -1) {
58+
SPDLOG_ERROR(
59+
"Failed to chmod socket {}: errno {}", addr.sun_path, errno);
60+
throw std::system_error(errno, std::generic_category());
61+
}
6362

64-
static constexpr int backlog = 50;
65-
if (::listen(sock_, backlog) == -1) {
66-
throw std::system_error(errno, std::generic_category());
67-
}
68-
SPDLOG_INFO("Started listening on {}", sv);
69-
} catch (const std::exception &e) {
70-
::close(sock_);
71-
throw;
63+
static constexpr int backlog = 50;
64+
if (::listen(sock_.get(), backlog) == -1) {
65+
throw std::system_error(errno, std::generic_category());
7266
}
67+
SPDLOG_INFO("Started listening on {}", sv);
7368
}
7469

7570
void acceptor::set_accept_timeout(std::chrono::seconds timeout)
7671
{
7772
struct timeval tv = {timeout.count(), 0};
78-
int const res = setsockopt(sock_, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));
73+
int const res =
74+
setsockopt(sock_.get(), SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));
7975
if (res == -1) {
8076
throw std::system_error(errno, std::generic_category());
8177
}
@@ -86,8 +82,9 @@ std::unique_ptr<base_socket> acceptor::accept()
8682
struct sockaddr_un addr {};
8783
socklen_t len = sizeof(addr);
8884

89-
// NOLINTNEXTLINE
90-
int s = ::accept(sock_, reinterpret_cast<struct sockaddr *>(&addr), &len);
85+
int s =
86+
// NOLINTNEXTLINE
87+
::accept(sock_.get(), reinterpret_cast<struct sockaddr *>(&addr), &len);
9188
if (s == -1) {
9289
if (errno == EINTR || errno == EAGAIN) {
9390
return {};

‎appsec/src/helper/network/acceptor.hpp‎

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
// (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc.
66
#pragma once
77

8+
#include "../utils.hpp"
89
#include "socket.hpp"
910
#include <chrono>
1011
#include <string_view>
@@ -28,35 +29,16 @@ namespace local {
2829

2930
class acceptor : public base_acceptor {
3031
public:
31-
explicit acceptor(int fd) : sock_{fd} {};
32+
explicit acceptor(owned_fd fd) : sock_{std::move(fd)} {};
3233
explicit acceptor(const std::string_view &sv);
3334
acceptor(const acceptor &) = delete;
3435
acceptor &operator=(const acceptor &) = delete;
3536

36-
acceptor(acceptor &&other) noexcept : sock_(other.sock_)
37-
{
38-
other.sock_ = -1;
39-
}
40-
41-
acceptor &operator=(acceptor &&other) noexcept
42-
{
43-
sock_ = other.sock_;
44-
other.sock_ = -1;
45-
return *this;
46-
}
47-
48-
~acceptor() override
49-
{
50-
if (sock_ != -1) {
51-
close(sock_);
52-
}
53-
}
54-
5537
void set_accept_timeout(std::chrono::seconds timeout) override;
5638
[[nodiscard]] std::unique_ptr<base_socket> accept() override;
5739

5840
private:
59-
int sock_{-1};
41+
owned_fd sock_;
6042
};
6143

6244
} // namespace local

‎appsec/src/helper/runner.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ std::unique_ptr<network::base_acceptor> acceptor_from_config(
4444
throw std::invalid_argument{
4545
"fd specified on config is invalid or no socket"};
4646
}
47-
return std::make_unique<network::local::acceptor>(fd);
47+
return std::make_unique<network::local::acceptor>(dds::owned_fd{fd});
4848
}
4949

5050
return std::make_unique<network::local::acceptor>(sock_path);

‎appsec/src/helper/utils.hpp‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,4 +65,31 @@ inline std::string strerror_ts(int errnum)
6565
return buf;
6666
}
6767

68+
class owned_fd {
69+
public:
70+
owned_fd() = default;
71+
explicit owned_fd(int fd) : fd_{fd} {}
72+
owned_fd(const owned_fd &) = delete;
73+
owned_fd &operator=(const owned_fd &) = delete;
74+
owned_fd(owned_fd &&other) noexcept : fd_{other.fd_} { other.fd_ = -1; }
75+
owned_fd &operator=(owned_fd &&other) noexcept
76+
{
77+
fd_ = other.fd_;
78+
other.fd_ = -1;
79+
return *this;
80+
}
81+
~owned_fd()
82+
{
83+
if (fd_ != -1) {
84+
::close(fd_);
85+
}
86+
}
87+
88+
int get() const { return fd_; }
89+
bool is_empty() const { return fd_ == -1; }
90+
91+
private:
92+
int fd_{-1};
93+
};
94+
6895
} // namespace dds

0 commit comments

Comments
 (0)