Skip to content

Commit 0914ee0

Browse files
stsoestsoe
authored
More clang-tidy fixes and disables (Xilinx#5205)
Disabled: // used commonly with __func__, etc +-hicpp-no-array-decay, // we do lots of bit manip +-hicpp-signed-bitwise, // not sure about these ones +-modernize-use-trailing-return-type, +-cppcoreguidelines-pro-bounds-pointer-arithmetic, // ert_packet is C-style union and used commonly +-cppcoreguidelines-pro-type-union-access, // used commonly with __func__, etc +-cppcoreguidelines-pro-bounds-array-to-pointer-decay, // we do lots of reinterpreting (e.g. xclbin structs) +-cppcoreguidelines-pro-type-reinterpret-cast, // data indexing in ert command objects is too common +-cppcoreguidelines-pro-bounds-constant-array-index Fixed xrt_ip.cpp, xrt_bo.cpp, xrt_kernel.cpp to build cleanly. Very tiduous, no errors detected, but good warnings in general.
1 parent 672b395 commit 0914ee0

File tree

4 files changed

+259
-128
lines changed

4 files changed

+259
-128
lines changed

‎src/.clang-tidy‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,26 @@
44
# clang-tidy "-checks=cert-*,bugprone-*,clang-analyzer-*,concurrency-*,cppcoreguidelines-*,hicpp-*,modernize-*" --dump-config
55
# clang-tidy looks for .clang-tidy file walking up the source directory structure to identify which checks to run
66
# One can customize checks in specific source directories by placing custom .clang-tidy in those source directories
7-
Checks: 'clang-diagnostic-*,clang-analyzer-*,cert-*,bugprone-*,clang-analyzer-*,concurrency-*,cppcoreguidelines-*,hicpp-*,modernize-*,-modernize-use-trailing-return-type,-cppcoreguidelines-pro-bounds-pointer-arithmetic'
7+
Checks: '
8+
clang-diagnostic-*,
9+
clang-analyzer-*,
10+
cert-*,
11+
bugprone-*,
12+
clang-analyzer-*,
13+
concurrency-*,
14+
cppcoreguidelines-*,
15+
hicpp-*,
16+
-hicpp-no-array-decay,
17+
-hicpp-signed-bitwise,
18+
modernize-*,
19+
-modernize-use-trailing-return-type,
20+
-cppcoreguidelines-pro-bounds-pointer-arithmetic,
21+
-cppcoreguidelines-pro-type-union-access,
22+
-cppcoreguidelines-pro-bounds-array-to-pointer-decay,
23+
-cppcoreguidelines-pro-type-reinterpret-cast,
24+
-cppcoreguidelines-pro-bounds-constant-array-index
25+
'
26+
827
WarningsAsErrors: ''
928
HeaderFilterRegex: ''
1029
AnalyzeTemporaryDtors: false
@@ -178,7 +197,7 @@ CheckOptions:
178197
- key: hicpp-no-malloc.Reallocations
179198
value: '::realloc'
180199
- key: hicpp-signed-bitwise.IgnorePositiveIntegerLiterals
181-
value: '0'
200+
value: '1'
182201
- key: hicpp-special-member-functions.AllowMissingMoveFunctions
183202
value: '0'
184203
- key: hicpp-special-member-functions.AllowSoleDefaultDtor

‎src/runtime_src/core/common/api/xrt_bo.cpp‎

Lines changed: 54 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ class bo_impl
144144
void
145145
get_bo_properties() const
146146
{
147-
xclBOProperties prop;
147+
xclBOProperties prop{};
148148
device->get_bo_properties(handle, &prop);
149149
addr = prop.paddr;
150150
grpid = prop.flags & XRT_BO_FLAGS_MEMIDX_MASK;
@@ -158,13 +158,14 @@ class bo_impl
158158
}
159159

160160
protected:
161-
std::shared_ptr<xrt_core::device> device;
162-
xclBufferHandle handle; // driver handle
163-
size_t size; // size of buffer
164-
mutable uint64_t addr = no_addr; // bo device address
165-
mutable int32_t grpid = no_group; // memory group index
166-
mutable bo::flags flags = no_flags; // flags per bo properties
167-
bool free_bo; // should dtor free bo
161+
// deliberately made protected, this is a file-scoped controlled API
162+
std::shared_ptr<xrt_core::device> device; // NOLINT
163+
xclBufferHandle handle; // NOLINT driver handle
164+
size_t size; // NOLINT size of buffer
165+
mutable uint64_t addr = no_addr; // NOLINT bo device address
166+
mutable int32_t grpid = no_group; // NOLINT memory group index
167+
mutable bo::flags flags = no_flags; // NOLINT flags per bo properties
168+
bool free_bo; // NOLINT should dtor free bo
168169

169170
public:
170171
explicit bo_impl(size_t sz)
@@ -178,7 +179,7 @@ class bo_impl
178179
bo_impl(xclDeviceHandle dhdl, xclBufferExportHandle ehdl)
179180
: device(xrt_core::get_userpf_device(dhdl)), handle(device->import_bo(ehdl)), free_bo(true)
180181
{
181-
xclBOProperties prop;
182+
xclBOProperties prop{};
182183
device->get_bo_properties(handle, &prop);
183184
size = prop.size;
184185
}
@@ -194,6 +195,11 @@ class bo_impl
194195
device->free_bo(handle);
195196
}
196197

198+
bo_impl(const bo_impl&) = delete;
199+
bo_impl(bo_impl&&) = delete;
200+
bo_impl& operator=(bo_impl&) = delete;
201+
bo_impl& operator=(bo_impl&&) = delete;
202+
197203
xclBufferHandle
198204
get_xcl_handle() const
199205
{
@@ -299,6 +305,7 @@ class bo_impl
299305
throw xrt_core::system_error(EINVAL, "No host side buffer in destination buffer");
300306

301307
// sync to src to ensure data integrity, logically const
308+
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast) // special case
302309
const_cast<bo_impl*>(src)->sync(XCL_BO_SYNC_BO_FROM_DEVICE, sz, src_offset);
303310

304311
// copy host side buffer
@@ -381,8 +388,8 @@ class buffer_ubuf : public bo_impl
381388
, ubuf(buf)
382389
{}
383390

384-
virtual void*
385-
get_hbuf() const
391+
void*
392+
get_hbuf() const override
386393
{
387394
return ubuf;
388395
}
@@ -402,8 +409,8 @@ class buffer_hbuf : public bo_impl
402409
: bo_impl(dhdl, bhdl, sz), hbuf(std::move(b))
403410
{}
404411

405-
virtual void*
406-
get_hbuf() const
412+
void*
413+
get_hbuf() const override
407414
{
408415
return hbuf.get();
409416
}
@@ -422,7 +429,7 @@ class buffer_kbuf : public bo_impl
422429
: bo_impl(dhdl, bhdl, sz), hbuf(device->map_bo(handle, true))
423430
{}
424431

425-
~buffer_kbuf()
432+
~buffer_kbuf() override
426433
{
427434
try {
428435
device->unmap_bo(handle, hbuf);
@@ -431,8 +438,13 @@ class buffer_kbuf : public bo_impl
431438
}
432439
}
433440

434-
virtual void*
435-
get_hbuf() const
441+
buffer_kbuf(const buffer_kbuf&) = delete;
442+
buffer_kbuf(buffer_kbuf&&) = delete;
443+
buffer_kbuf& operator=(buffer_kbuf&) = delete;
444+
buffer_kbuf& operator=(buffer_kbuf&&) = delete;
445+
446+
void*
447+
get_hbuf() const override
436448
{
437449
return hbuf;
438450
}
@@ -458,7 +470,7 @@ class buffer_import : public bo_impl
458470
}
459471
}
460472

461-
~buffer_import()
473+
~buffer_import() override
462474
{
463475
try {
464476
device->unmap_bo(handle, hbuf);
@@ -467,14 +479,19 @@ class buffer_import : public bo_impl
467479
}
468480
}
469481

470-
virtual bool
471-
is_imported() const
482+
buffer_import(const buffer_import&) = delete;
483+
buffer_import(buffer_import&&) = delete;
484+
buffer_import& operator=(buffer_import&) = delete;
485+
buffer_import& operator=(buffer_import&&) = delete;
486+
487+
bool
488+
is_imported() const override
472489
{
473490
return true;
474491
}
475492

476-
virtual void*
477-
get_hbuf() const
493+
void*
494+
get_hbuf() const override
478495
{
479496
if (!hbuf)
480497
throw xrt_core::system_error(std::errc::bad_address, "No host memory for imported buffer");
@@ -491,8 +508,8 @@ class buffer_dbuf : public bo_impl
491508
: bo_impl(dhdl, bhdl, sz)
492509
{}
493510

494-
virtual void*
495-
get_hbuf() const
511+
void*
512+
get_hbuf() const override
496513
{
497514
throw xrt_core::error(-EINVAL, "device only buffer has no host buffer");
498515
}
@@ -548,16 +565,16 @@ class buffer_nodma : public bo_impl
548565
}
549566
}
550567

551-
virtual void*
552-
get_hbuf() const
568+
void*
569+
get_hbuf() const override
553570
{
554571
return m_ubuf ? m_ubuf : m_host_only.get_hbuf();
555572
}
556573

557574
// sync is M2M copy between host and device bo
558575
// nodma is guaranteed to have M2M
559576
void
560-
sync(xclBOSyncDirection dir, size_t sz, size_t offset)
577+
sync(xclBOSyncDirection dir, size_t sz, size_t offset) override
561578
{
562579
if (dir == XCL_BO_SYNC_BO_TO_DEVICE) {
563580
sync_to_hbuf(sz, offset);
@@ -572,7 +589,7 @@ class buffer_nodma : public bo_impl
572589
}
573590

574591
void
575-
copy(const bo_impl* src, size_t sz, size_t src_offset, size_t dst_offset)
592+
copy(const bo_impl* src, size_t sz, size_t src_offset, size_t dst_offset) override
576593
{
577594
// Copy src device bo to dst (this) device bo
578595
bo_impl::copy(src, sz, src_offset, dst_offset);
@@ -604,26 +621,26 @@ class buffer_sub : public bo_impl
604621
throw xrt_core::error(-EINVAL, "sub buffer size and offset");
605622
}
606623

607-
virtual size_t
608-
get_offset() const
624+
size_t
625+
get_offset() const override
609626
{
610627
return offset;
611628
}
612629

613-
virtual void*
614-
get_hbuf() const
630+
void*
631+
get_hbuf() const override
615632
{
616633
return hbuf;
617634
}
618635

619-
virtual bool
620-
is_sub_buffer() const
636+
bool
637+
is_sub_buffer() const override
621638
{
622639
return true;
623640
}
624641

625-
virtual uint64_t
626-
get_address() const
642+
uint64_t
643+
get_address() const override
627644
{
628645
return bo_impl::get_address() + offset;
629646
}
@@ -723,7 +740,8 @@ alloc_dbuf(xclDeviceHandle dhdl, size_t sz, xrtBufferFlags, xrtMemoryGroup grp)
723740
static std::shared_ptr<xrt::bo_impl>
724741
alloc_nodma(xclDeviceHandle dhdl, size_t sz, xrtBufferFlags, xrtMemoryGroup grp, void* userptr=nullptr)
725742
{
726-
if (sz % 64)
743+
constexpr size_t align = 64;
744+
if (sz % align)
727745
throw xrt_core::error(EINVAL, "Invalid buffer size '" + std::to_string(sz) +
728746
"', must be multiple of 64 bytes for NoDMA platforms");
729747

‎src/runtime_src/core/common/api/xrt_ip.cpp‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939

4040
namespace {
4141

42-
constexpr size_t operator"" _kb(unsigned long long v) { return 1024u * v; }
42+
constexpr size_t operator"" _kb(unsigned long long v) { return 1024u * v; } //NOLINT
4343

4444
inline bool
4545
is_sw_emulation()
@@ -86,6 +86,11 @@ class ip::interrupt_impl
8686
}
8787
}
8888

89+
interrupt_impl(const interrupt_impl&) = delete;
90+
interrupt_impl(interrupt_impl&&) = delete;
91+
interrupt_impl& operator=(interrupt_impl&) = delete;
92+
interrupt_impl& operator=(interrupt_impl&&) = delete;
93+
8994
void
9095
enable()
9196
{
@@ -121,11 +126,12 @@ class ip_impl
121126
xrt::uuid xclbin_uuid; //
122127
unsigned int idx; // index of ip per driver
123128
const ip_data* ip;
124-
uint64_t address; // base address of ip
125129
uint64_t size; // address range of ip, To-Be-Computed
126130

127-
ip_context(std::shared_ptr<xrt_core::device> dev, const xrt::uuid& xid, const std::string& nm)
128-
: device(std::move(dev)), xclbin_uuid(xid), size(64_kb)
131+
ip_context(std::shared_ptr<xrt_core::device> dev, xrt::uuid xid, const std::string& nm)
132+
: device(std::move(dev)),
133+
xclbin_uuid(std::move(xid)),
134+
size(64_kb) // NOLINT(cppcoreguidelines-avoid-magic-numbers)
129135
{
130136
auto ip_section = device->get_axlf_section(IP_LAYOUT, xclbin_uuid);
131137
if (!ip_section.first)
@@ -155,6 +161,11 @@ class ip_impl
155161
device->close_context(xclbin_uuid.get(), idx);
156162
}
157163

164+
ip_context(const ip_context&) = delete;
165+
ip_context(ip_context&&) = delete;
166+
ip_context& operator=(ip_context&) = delete;
167+
ip_context& operator=(ip_context&&) = delete;
168+
158169
unsigned int
159170
get_idx() const
160171
{
@@ -215,6 +226,11 @@ class ip_impl
215226
XRT_DEBUGF("ip_impl::~ip_impl(%d)\n" , uid);
216227
}
217228

229+
ip_impl(const ip_impl&) = delete;
230+
ip_impl(ip_impl&&) = delete;
231+
ip_impl& operator=(ip_impl&) = delete;
232+
ip_impl& operator=(ip_impl&&) = delete;
233+
218234
uint32_t
219235
read_register(uint32_t offset) const
220236
{
@@ -242,6 +258,7 @@ class ip_impl
242258
{
243259
auto intr = interrupt.lock();
244260
if (!intr)
261+
// NOLINTNEXTLINE(modernize-make-shared) used in weak_ptr
245262
interrupt = intr = std::shared_ptr<ip::interrupt_impl>(new ip::interrupt_impl(device, ipctx.get_idx()));
246263

247264
return intr;

0 commit comments

Comments
 (0)