Skip to content

Commit d53251f

Browse files
committed
Fix various small issues found by clang-tidy and disable some warnings
1 parent b5460ba commit d53251f

12 files changed

Lines changed: 48 additions & 35 deletions

.clang-tidy

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
---
2-
Checks: '*,-android-cloexec-*,-cppcoreguidelines-avoid-magic-numbers,-cppcoreguidelines-avoid-non-const-global-variables,-cppcoreguidelines-owning-memory,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-cppcoreguidelines-pro-type-cstyle-cast,-cppcoreguidelines-pro-type-reinterpret-cast,-cppcoreguidelines-pro-type-static-cast-downcast,-cppcoreguidelines-pro-type-vararg,-fuchsia-*,-google-readability-casting,-google-readability-todo,-hicpp-named-parameter,-hicpp-no-array-decay,-hicpp-vararg,-llvm-include-order,-llvm-header-guard,-llvmlibc-*,-misc-no-recursion,-modernize-use-trailing-return-type,-readability-implicit-bool-conversion,-readability-named-parameter,-readability-magic-numbers'
2+
Checks: '*,-android-cloexec-*,-cert-err58-cpp,-cppcoreguidelines-avoid-magic-numbers,-cppcoreguidelines-avoid-non-const-global-variables,-cppcoreguidelines-owning-memory,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-cppcoreguidelines-pro-type-cstyle-cast,-cppcoreguidelines-pro-type-reinterpret-cast,-cppcoreguidelines-pro-type-static-cast-downcast,-cppcoreguidelines-pro-type-vararg,-fuchsia-*,-google-readability-casting,-google-readability-todo,-hicpp-named-parameter,-hicpp-no-array-decay,-hicpp-vararg,-llvm-include-order,-llvm-header-guard,-llvmlibc-*,-misc-no-recursion,-modernize-use-nodiscard,-modernize-use-trailing-return-type,-readability-implicit-bool-conversion,-readability-named-parameter,-readability-magic-numbers'
33
#
44
# cppcoreguidelines-pro-type-cstyle-cast
55
# google-build-using-namespace
@@ -13,6 +13,10 @@ Checks: '*,-android-cloexec-*,-cppcoreguidelines-avoid-magic-numbers,-cppcoregui
1313
# android-cloexec-*
1414
# O_CLOEXEC isn't available on Windows
1515
#
16+
# cert-err58-cpp
17+
# There are many of these in the test code, most of them from the Catch
18+
# framework. Difficult to avoid.
19+
#
1620
# cppcoreguidelines-avoid-magic-numbers
1721
# readability-magic-numbers
1822
# We have a lot of these and should probably at least fix some. But remove
@@ -45,6 +49,10 @@ Checks: '*,-android-cloexec-*,-cppcoreguidelines-avoid-magic-numbers,-cppcoregui
4549
# misc-no-recursion
4650
# Nothing wrong with recursion
4751
#
52+
# modernize-use-nodiscard
53+
# We have a lot of these. Remove it for the time being because there are
54+
# too many to fix quickly. (TODO)
55+
#
4856
# modernize-use-trailing-return-type
4957
# We are not that modern...
5058
#

src/flex-table-column.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ static std::array<column_type_lookup, 25> const column_types = {
5353
static table_column_type
5454
get_column_type_from_string(std::string const &type)
5555
{
56+
// Because it doesn't work with MSVC:
57+
// NOLINTNEXTLINE(llvm-qualified-auto,readability-qualified-auto)
5658
auto const it =
5759
std::find_if(std::begin(column_types), std::end(column_types),
5860
[&type](column_type_lookup name_type) {

src/format.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#define FMT_HEADER_ONLY
1414
#include <fmt/format.h>
1515

16-
using namespace fmt::literals; // NOLINT(google-global-names-in-headers)
16+
// NOLINTNEXTLINE(google-global-names-in-headers,google-build-using-namespace)
17+
using namespace fmt::literals;
1718

1819
#endif // OSM2PGSQL_FORMAT_HPP

src/middle-pgsql.cpp

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,12 @@
1515
* emit the final geometry-enabled output formats
1616
*/
1717

18-
#include <stdexcept>
19-
#include <unordered_map>
20-
2118
#include <cassert>
2219
#include <cstdlib>
23-
#include <functional>
2420
#include <memory>
21+
#include <stdexcept>
22+
#include <string>
23+
#include <unordered_map>
2524
#include <utility>
2625

2726
#include <osmium/builder/osm_object_builder.hpp>
@@ -643,9 +642,9 @@ void middle_pgsql_t::stop()
643642
} else if (!m_options->append) {
644643
// Building the indexes takes time, so do it asynchronously.
645644
for (auto &table : m_tables) {
646-
table.task_set(thread_pool().submit(
647-
std::bind(&middle_pgsql_t::table_desc::build_index, &table,
648-
m_options->database_options.conninfo())));
645+
table.task_set(thread_pool().submit([&]() {
646+
table.build_index(m_options->database_options.conninfo());
647+
}));
649648
}
650649
}
651650
}

src/options.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ struct option const long_options[] = {
101101
{"with-forward-dependencies", required_argument, nullptr, 217},
102102
{nullptr, 0, nullptr, 0}};
103103

104-
static void long_usage(char const *arg0, bool verbose)
104+
void long_usage(char const *arg0, bool verbose)
105105
{
106106
char const *const name = program_name(arg0);
107107

src/pgsql.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,17 +89,17 @@ void pg_conn_t::copy_data(std::string const &sql,
8989
log_sql_data("Copy data to '{}':\n{}", context, sql);
9090
int const r = PQputCopyData(m_conn.get(), sql.c_str(), (int)sql.size());
9191

92-
if (r == 1) {
93-
return; // success
94-
}
95-
9692
switch (r) {
9793
case 0: // need to wait for write ready
9894
log_error("{} - COPY unexpectedly busy", context);
9995
break;
96+
case 1: // success
97+
return;
10098
case -1: // error occurred
10199
log_error("{} - error on COPY: {}", context, error_msg());
102100
break;
101+
default: // unexpected result
102+
break;
103103
}
104104

105105
if (sql.size() < 1100) {

src/tagtransform-c.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -322,15 +322,12 @@ bool c_tagtransform_t::filter_rel_member_tags(
322322
out_tags.add_tag_if_not_exists("nwn_ref", *relref);
323323
}
324324
}
325-
} else if (is_boundary) {
325+
} else if (is_boundary || (is_multipolygon && out_tags.contains("boundary"))) {
326326
/* Boundaries will get converted into multiple geometries:
327327
- Linear features will end up in the line and roads tables (useful for admin boundaries)
328328
- Polygon features also go into the polygon table (useful for national_forests)
329329
The edges of the polygon also get treated as linear fetaures allowing these to be rendered seperately. */
330330
*make_boundary = true;
331-
} else if (is_multipolygon && out_tags.contains("boundary")) {
332-
/* Treat type=multipolygon exactly like type=boundary if it has a boundary tag. */
333-
*make_boundary = true;
334331
} else if (is_multipolygon) {
335332
*make_polygon = true;
336333
}

src/wkb.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ enum wkb_byte_order_type_t : uint8_t
4646
};
4747

4848
template <typename T>
49-
static void str_push(std::string *data, T value)
49+
void str_push(std::string *data, T value)
5050
{
5151
data->append(reinterpret_cast<char const *const>(&value), sizeof(T));
5252
}
@@ -58,7 +58,7 @@ static void str_push(std::string *data, T value)
5858
*
5959
* \pre \code data != nullptr \endcode
6060
*/
61-
static void write_header(std::string *data, geometry_type type, uint32_t srid)
61+
void write_header(std::string *data, geometry_type type, uint32_t srid)
6262
{
6363
str_push(data, Endian);
6464
if (srid) {
@@ -76,12 +76,12 @@ static void write_header(std::string *data, geometry_type type, uint32_t srid)
7676
*
7777
* \pre \code data != nullptr \endcode
7878
*/
79-
static void write_length(std::string *data, std::size_t length)
79+
void write_length(std::string *data, std::size_t length)
8080
{
8181
str_push(data, static_cast<uint32_t>(length));
8282
}
8383

84-
static void write_points(std::string *data, geom::point_list_t const &points)
84+
void write_points(std::string *data, geom::point_list_t const &points)
8585
{
8686
write_length(data, points.size());
8787
for (auto const &point : points) {
@@ -90,7 +90,7 @@ static void write_points(std::string *data, geom::point_list_t const &points)
9090
}
9191
}
9292

93-
static void write_linestring(std::string *data, geom::linestring_t const &geom,
93+
void write_linestring(std::string *data, geom::linestring_t const &geom,
9494
uint32_t srid)
9595
{
9696
assert(data);
@@ -99,7 +99,7 @@ static void write_linestring(std::string *data, geom::linestring_t const &geom,
9999
write_points(data, geom);
100100
}
101101

102-
static void write_polygon(std::string *data, geom::polygon_t const &geom,
102+
void write_polygon(std::string *data, geom::polygon_t const &geom,
103103
uint32_t srid)
104104
{
105105
assert(data);
@@ -343,7 +343,7 @@ class ewkb_parser_t
343343
{
344344
check_bytes(sizeof(double) * 2);
345345

346-
std::array<double, 2> data;
346+
std::array<double, 2> data{};
347347
std::memcpy(&data[0], m_it, sizeof(double) * 2);
348348
m_it += sizeof(double) * 2;
349349

tests/test-expire-tiles.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
#include <catch.hpp>
1111

12+
#include <random>
1213
#include <set>
1314

1415
#include "expire-tiles.hpp"
@@ -43,13 +44,18 @@ struct tile_output_set
4344

4445
static tile_output_set generate_random(uint32_t zoom, size_t count)
4546
{
47+
// Use a random device with a fixed seed. We don't really care about
48+
// the quality of random numbers here, we just need to generate valid
49+
// OSM test data. The fixed seed ensures that the results are
50+
// reproducible.
51+
// NOLINTNEXTLINE(cert-msc32-c,cert-msc51-cpp)
52+
static std::mt19937_64 rng{47382};
53+
54+
std::uniform_int_distribution<uint32_t> dist{0, (1U << zoom) - 1U};
4655
tile_output_set set;
47-
auto const cmask = (1UL << zoom) - 1U;
4856

4957
do {
50-
set.output_dirty_tile(
51-
tile_t{zoom, static_cast<uint32_t>(rand() & cmask),
52-
static_cast<uint32_t>(rand() & cmask)});
58+
set.output_dirty_tile(tile_t{zoom, dist(rng), dist(rng)});
5359
} while (set.tiles.size() < count);
5460

5561
return set;

tests/test-middle.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -857,17 +857,17 @@ TEMPLATE_TEST_CASE("middle: add relation with attributes", "",
857857

858858
// Create some relations we'll use for the tests.
859859
test_buffer_t buffer;
860-
auto &relation30 = buffer.add_relation(
860+
auto const &relation30 = buffer.add_relation(
861861
"r30 v123 c456 i789 t2009-02-13T23:31:30Z Mw10@outer,w11@inner "
862862
"Ttype=multipolygon,name=Penguin_Park");
863863

864864
// The same relation but with default attributes.
865-
auto &relation30_no_attr = buffer.add_relation(
865+
auto const &relation30_no_attr = buffer.add_relation(
866866
"r30 Mw10@outer,w11@inner Ttype=multipolygon,name=Penguin_Park");
867867

868868
// The same relation but with attributes in tags.
869869
// The order of the tags is important here!
870-
auto &relation30_attr_tags = buffer.add_relation(
870+
auto const &relation30_attr_tags = buffer.add_relation(
871871
"r30 Mw10@outer,w11@inner "
872872
"Ttype=multipolygon,name=Penguin_Park,osm_user=,osm_uid=789,"
873873
"osm_version=123,osm_timestamp=2009-02-13T23:31:30Z,osm_changeset=456");

0 commit comments

Comments
 (0)