Various code cleanups

joto · joto · commit ebe1b9df7f6f · 2020-11-25T21:56:51.000+01:00
Various code cleanups based on issues found by clang-tidy.
diff --git a/src/db-check.cpp b/src/db-check.cpp
@@ -16,7 +16,7 @@ static bool has_table(pg_conn_t &db_connection, std::string const &schema,
                      "  WHERE schemaname='{}' AND tablename='{}'"_format(
                          schema.empty() ? "public" : schema, table);
     auto const res = db_connection.query(PGRES_TUPLES_OK, sql);
-    auto const num = res.get_value(0, 0);
+    char const *const num = res.get_value(0, 0);
 
     return num[0] == '1' && num[1] == '\0';
 }
diff --git a/src/db-copy-mgr.hpp b/src/db-copy-mgr.hpp
@@ -27,7 +27,7 @@ class db_copy_mgr_t
      */
     void new_line(std::shared_ptr<db_target_descr_t> const &table)
     {
-        if (!m_current || !m_current->target->same_copy_target(*table.get())) {
+        if (!m_current || !m_current->target->same_copy_target(*table)) {
             if (m_current) {
                 m_processor->add_buffer(std::move(m_current));
             }
diff --git a/src/db-copy.cpp b/src/db-copy.cpp
@@ -160,7 +160,7 @@ void db_copy_thread_t::thread_t::operator()()
 void db_copy_thread_t::thread_t::write_to_db(db_cmd_copy_t *buffer)
 {
     if (buffer->has_deletables() ||
-        (m_inflight && !buffer->target->same_copy_target(*m_inflight.get()))) {
+        (m_inflight && !buffer->target->same_copy_target(*m_inflight))) {
         finish_copy();
     }
 
diff --git a/src/expire-tiles.hpp b/src/expire-tiles.hpp
@@ -20,9 +20,8 @@ class parser_t;
  */
 struct xy_coord_t
 {
-    uint32_t x;
-    uint32_t y;
-    xy_coord_t() : x(0), y(0) {}
+    uint32_t x = 0;
+    uint32_t y = 0;
 };
 
 /**
@@ -33,7 +32,7 @@ class tile_output_t
     FILE *outfile;
 
 public:
-    tile_output_t(char const *filename);
+    explicit tile_output_t(char const *filename);
 
     ~tile_output_t();
 
diff --git a/src/flex-table-column.cpp b/src/flex-table-column.cpp
@@ -13,31 +13,31 @@ struct column_type_lookup
     table_column_type type;
 };
 
-static column_type_lookup const column_types[] = {
-    {"sql", table_column_type::sql},
-    {"text", table_column_type::text},
-    {"boolean", table_column_type::boolean},
-    {"bool", table_column_type::boolean},
-    {"int2", table_column_type::int2},
-    {"smallint", table_column_type::int2},
-    {"int4", table_column_type::int4},
-    {"int", table_column_type::int4},
-    {"integer", table_column_type::int4},
-    {"int8", table_column_type::int8},
-    {"bigint", table_column_type::int8},
-    {"real", table_column_type::real},
-    {"hstore", table_column_type::hstore},
-    {"direction", table_column_type::direction},
-    {"geometry", table_column_type::geometry},
-    {"point", table_column_type::point},
-    {"linestring", table_column_type::linestring},
-    {"polygon", table_column_type::polygon},
-    {"multipoint", table_column_type::multipoint},
-    {"multilinestring", table_column_type::multilinestring},
-    {"multipolygon", table_column_type::multipolygon},
-    {"area", table_column_type::area},
-    {"id_type", table_column_type::id_type},
-    {"id_num", table_column_type::id_num}};
+static std::array<column_type_lookup, 24> const column_types = {
+    {{"sql", table_column_type::sql},
+     {"text", table_column_type::text},
+     {"boolean", table_column_type::boolean},
+     {"bool", table_column_type::boolean},
+     {"int2", table_column_type::int2},
+     {"smallint", table_column_type::int2},
+     {"int4", table_column_type::int4},
+     {"int", table_column_type::int4},
+     {"integer", table_column_type::int4},
+     {"int8", table_column_type::int8},
+     {"bigint", table_column_type::int8},
+     {"real", table_column_type::real},
+     {"hstore", table_column_type::hstore},
+     {"direction", table_column_type::direction},
+     {"geometry", table_column_type::geometry},
+     {"point", table_column_type::point},
+     {"linestring", table_column_type::linestring},
+     {"polygon", table_column_type::polygon},
+     {"multipoint", table_column_type::multipoint},
+     {"multilinestring", table_column_type::multilinestring},
+     {"multipolygon", table_column_type::multipolygon},
+     {"area", table_column_type::area},
+     {"id_type", table_column_type::id_type},
+     {"id_num", table_column_type::id_num}}};
 
 static table_column_type
 get_column_type_from_string(std::string const &type) noexcept
diff --git a/src/flex-table.hpp b/src/flex-table.hpp
@@ -32,7 +32,7 @@ class flex_table_t
         permanent
     };
 
-    flex_table_t(std::string name) : m_name(std::move(name)) {}
+    explicit flex_table_t(std::string name) : m_name(std::move(name)) {}
 
     std::string const &name() const noexcept { return m_name; }
 
diff --git a/src/gazetteer-style.cpp b/src/gazetteer-style.cpp
@@ -290,7 +290,7 @@ void gazetteer_style_t::process_tags(osmium::OSMObject const &o)
     bool has_postcode = false;
     bool has_country = false;
     char const *place = nullptr;
-    flag_t place_flag;
+    flag_t place_flag = 0;
     bool address_point = false;
     bool interpolation = false;
     bool admin_boundary = false;
@@ -340,7 +340,7 @@ void gazetteer_style_t::process_tags(osmium::OSMObject const &o)
         }
 
         if (flag & SF_ADDRESS) {
-            char const *addr_key;
+            char const *addr_key = nullptr;
             if (std::strncmp(k, "addr:", 5) == 0) {
                 addr_key = k + 5;
             } else if (std::strncmp(k, "is_in:", 6) == 0) {
diff --git a/src/geom-transform.cpp b/src/geom-transform.cpp
@@ -82,7 +82,7 @@ bool geom_transform_area_t::set_param(char const *name, lua_State *lua_state)
         return false;
     }
 
-    auto const val = lua_tostring(lua_state, -1);
+    char const *const val = lua_tostring(lua_state, -1);
 
     if (!val) {
         throw std::runtime_error{
diff --git a/src/middle-pgsql.cpp b/src/middle-pgsql.cpp
@@ -180,7 +180,7 @@ void pgsql_parse_members(char const *string, osmium::memory::Buffer &buffer,
 
     while (*string != '}') {
         char type = string[0];
-        char *endp;
+        char *endp = nullptr;
         osmid_t id = std::strtoll(string + 1, &endp, 10);
         // String points to the comma */
         string = decode_upto(endp + 1, role);
@@ -198,7 +198,7 @@ void pgsql_parse_nodes(char const *string, osmium::memory::Buffer &buffer,
     if (*string++ == '{') {
         osmium::builder::WayNodeListBuilder wnl_builder{buffer, &builder};
         while (*string != '}') {
-            char *ptr;
+            char *ptr = nullptr;
             wnl_builder.add_node_ref(std::strtoll(string, &ptr, 10));
             string = ptr;
             if (*string == ',') {
diff --git a/src/middle-pgsql.hpp b/src/middle-pgsql.hpp
@@ -54,7 +54,7 @@ struct table_sql {
 
 struct middle_pgsql_t : public slim_middle_t
 {
-    middle_pgsql_t(options_t const *options);
+    explicit middle_pgsql_t(options_t const *options);
 
     void start() override;
     void stop(thread_pool_t &pool) override;
@@ -79,7 +79,7 @@ struct middle_pgsql_t : public slim_middle_t
     class table_desc
     {
     public:
-        table_desc() {}
+        table_desc() = default;
         table_desc(options_t const &options, table_sql const &ts);
 
         char const *name() const { return m_copy_target->name.c_str(); }
diff --git a/src/node-ram-cache.cpp b/src/node-ram-cache.cpp
@@ -234,11 +234,9 @@ void node_ram_cache::set_dense(osmid_t id, osmium::Location location)
          * will break the invariant. However, it will work fine if all the
          * nodes come in numerical order, which is the common case */
 
-        int expectedpos;
+        int expectedpos = 0;
         if ((usedBlocks < maxBlocks) && (cacheUsed < cacheSize)) {
             expectedpos = usedBlocks - 1;
-        } else {
-            expectedpos = 0;
         }
 
         if (queue[expectedpos] != &blocks[block]) {
diff --git a/src/osmdata.cpp b/src/osmdata.cpp
@@ -57,7 +57,7 @@ void osmdata_t::node_add(osmium::Node const &node) const
     m_mid->node_set(node);
 
     if (m_with_extra_attrs || !node.tags().empty()) {
-        for (auto &out : m_outs) {
+        for (auto const &out : m_outs) {
             out->node_add(node);
         }
     }
@@ -68,7 +68,7 @@ void osmdata_t::way_add(osmium::Way *way) const
     m_mid->way_set(*way);
 
     if (m_with_extra_attrs || !way->tags().empty()) {
-        for (auto &out : m_outs) {
+        for (auto const &out : m_outs) {
             out->way_add(way);
         }
     }
@@ -79,7 +79,7 @@ void osmdata_t::relation_add(osmium::Relation const &rel) const
     m_mid->relation_set(rel);
 
     if (m_with_extra_attrs || !rel.tags().empty()) {
-        for (auto &out : m_outs) {
+        for (auto const &out : m_outs) {
             out->relation_add(rel);
         }
     }
@@ -92,7 +92,7 @@ void osmdata_t::node_modify(osmium::Node const &node) const
     slim.node_delete(node.id());
     slim.node_set(node);
 
-    for (auto &out : m_outs) {
+    for (auto const &out : m_outs) {
         out->node_modify(node);
     }
 
@@ -106,7 +106,7 @@ void osmdata_t::way_modify(osmium::Way *way) const
     slim.way_delete(way->id());
     slim.way_set(*way);
 
-    for (auto &out : m_outs) {
+    for (auto const &out : m_outs) {
         out->way_modify(way);
     }
 
@@ -117,21 +117,21 @@ void osmdata_t::relation_modify(osmium::Relation const &rel) const
 {
     auto &slim = slim_middle();
 
-    for (auto &out : m_outs) {
+    for (auto const &out : m_outs) {
         out->select_relation_members(rel.id());
     }
 
     slim.relation_delete(rel.id());
     slim.relation_set(rel);
 
-    for (auto &out : m_outs) {
+    for (auto const &out : m_outs) {
         out->relation_modify(rel);
     }
 }
 
 void osmdata_t::node_delete(osmid_t id) const
 {
-    for (auto &out : m_outs) {
+    for (auto const &out : m_outs) {
         out->node_delete(id);
     }
 
@@ -140,7 +140,7 @@ void osmdata_t::node_delete(osmid_t id) const
 
 void osmdata_t::way_delete(osmid_t id) const
 {
-    for (auto &out : m_outs) {
+    for (auto const &out : m_outs) {
         out->way_delete(id);
     }
 
@@ -149,7 +149,7 @@ void osmdata_t::way_delete(osmid_t id) const
 
 void osmdata_t::relation_delete(osmid_t id) const
 {
-    for (auto &out : m_outs) {
+    for (auto const &out : m_outs) {
         out->relation_delete(id);
     }
 
@@ -158,7 +158,7 @@ void osmdata_t::relation_delete(osmid_t id) const
 
 void osmdata_t::start() const
 {
-    for (auto &out : m_outs) {
+    for (auto const &out : m_outs) {
         out->start();
     }
 }
@@ -292,7 +292,7 @@ class multithreaded_processor
     /// Runs in a worker thread: Update progress display once per second.
     static void print_stats(idlist_t *queue, std::mutex *mutex)
     {
-        size_t queue_size;
+        std::size_t queue_size = 0;
         do {
             mutex->lock();
             queue_size = queue->size();
diff --git a/src/osmium-builder.cpp b/src/osmium-builder.cpp
@@ -273,7 +273,7 @@ osmium_builder_t::get_wkb_multiline(osmium::memory::Buffer const &ways,
             do {
                 auto &conn = conns[cur];
                 assert(std::get<1>(conn));
-                auto &nl = std::get<1>(conn)->nodes();
+                auto const &nl = std::get<1>(conn)->nodes();
                 bool const skip_first = prev != NOCONN;
                 bool const forward = std::get<0>(conn) == prev;
                 prev = cur;
diff --git a/src/osmtypes.hpp b/src/osmtypes.hpp
@@ -29,7 +29,7 @@ struct member
     osmid_t id;
     std::string role;
 
-    operator osmium::builder::attr::member_type const() const
+    explicit operator osmium::builder::attr::member_type const() const
     {
         return osmium::builder::attr::member_type(type, id, role.c_str());
     }
@@ -41,8 +41,6 @@ struct member
 
 struct memberlist_t : public std::vector<member>
 {
-    memberlist_t() {}
-
     explicit memberlist_t(osmium::RelationMemberList const &list)
     {
         for (auto const &m : list) {
diff --git a/src/output-flex.cpp b/src/output-flex.cpp
@@ -275,7 +275,7 @@ void write_integer(db_copy_mgr_t<db_deleter_by_type_and_id_t> *copy_mgr,
         return;
     }
 
-    char *end;
+    char *end = nullptr;
     errno = 0;
     auto const value = std::strtoll(str, &end, 10);
 
@@ -658,7 +658,8 @@ void output_flex_t::setup_id_columns(flex_table_t *table)
         table->set_id_type(osmium::item_type::undefined);
         lua_getfield(lua_state(), -2, "type_column");
         if (lua_isstring(lua_state(), -1)) {
-            auto const column_name = lua_tolstring(lua_state(), -1, nullptr);
+            std::string const column_name =
+                lua_tolstring(lua_state(), -1, nullptr);
             check_name(column_name, "column");
             auto &column = table->add_column(column_name, "id_type");
             column.set_not_null();
diff --git a/src/output-pgsql.cpp b/src/output-pgsql.cpp
@@ -75,8 +75,8 @@ void output_pgsql_t::pending_way(osmid_t id)
         pgsql_delete_way_from_output(id);
 
         taglist_t outtags;
-        int polygon;
-        int roads;
+        int polygon = 0;
+        int roads = 0;
         auto &way = buffer.get<osmium::Way>(0);
         if (!m_tagtransform->filter_tags(way, &polygon, &roads, outtags)) {
             auto nnodes = m_mid->nodes_get_list(&(way.nodes()));
diff --git a/src/table.hpp b/src/table.hpp
@@ -18,8 +18,8 @@ class table_t
 {
 public:
     table_t(std::string const &name, std::string const &type,
-            columns_t const &columns, hstores_t const &hstore_columns,
-            int const srid, bool const append, hstore_column hstore_mode,
+            columns_t const &columns, hstores_t const &hstore_columns, int srid,
+            bool append, hstore_column hstore_mode,
             std::shared_ptr<db_copy_thread_t> const &copy_thread,
             std::string const &schema);
     table_t(table_t const &other,
@@ -32,7 +32,7 @@ class table_t
     void sync();
 
     void write_row(osmid_t id, taglist_t const &tags, std::string const &geom);
-    void delete_row(osmid_t const id);
+    void delete_row(osmid_t id);
 
     // interface for retrieving well known binary geometry from the table
     class wkb_reader
diff --git a/src/util.cpp b/src/util.cpp
@@ -7,7 +7,9 @@ std::string human_readable_duration(uint64_t seconds)
 {
     if (seconds < 60) {
         return "{}s"_format(seconds);
-    } else if (seconds < (60 * 60)) {
+    }
+
+    if (seconds < (60 * 60)) {
         return "{}s ({}m {}s)"_format(seconds, seconds / 60, seconds % 60);
     }
 
diff --git a/src/util.hpp b/src/util.hpp
diff --git a/tests/test-parse-osmium.cpp b/tests/test-parse-osmium.cpp

Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,7 @@ static bool has_table(pg_conn_t &db_connection, std::string const &schema,`
`16`	`16`	`" WHERE schemaname='{}' AND tablename='{}'"_format(`
`17`	`17`	`schema.empty() ? "public" : schema, table);`
`18`	`18`	`auto const res = db_connection.query(PGRES_TUPLES_OK, sql);`
`19`		`- auto const num = res.get_value(0, 0);`
	`19`	`+ char const *const num = res.get_value(0, 0);`
`20`	`20`
`21`	`21`	`return num[0] == '1' && num[1] == '\0';`
`22`	`22`	`}`
Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,7 @@ class db_copy_mgr_t`
`27`	`27`	`*/`
`28`	`28`	`void new_line(std::shared_ptr<db_target_descr_t> const &table)`
`29`	`29`	`{`
`30`		`- if (!m_current \|\| !m_current->target->same_copy_target(*table.get())) {`
	`30`	`+ if (!m_current \|\| !m_current->target->same_copy_target(*table)) {`
`31`	`31`	`if (m_current) {`
`32`	`32`	`m_processor->add_buffer(std::move(m_current));`
`33`	`33`	`}`
Original file line number	Diff line number	Diff line change
`@@ -160,7 +160,7 @@ void db_copy_thread_t::thread_t::operator()()`
`160`	`160`	`void db_copy_thread_t::thread_t::write_to_db(db_cmd_copy_t *buffer)`
`161`	`161`	`{`
`162`	`162`	`if (buffer->has_deletables() \|\|`
`163`		`- (m_inflight && !buffer->target->same_copy_target(*m_inflight.get()))) {`
	`163`	`+ (m_inflight && !buffer->target->same_copy_target(*m_inflight))) {`
`164`	`164`	`finish_copy();`
`165`	`165`	`}`
`166`	`166`
Original file line number	Diff line number	Diff line change
`@@ -82,7 +82,7 @@ bool geom_transform_area_t::set_param(char const name, lua_State lua_state)`
`82`	`82`	`return false;`
`83`	`83`	`}`
`84`	`84`
`85`		`- auto const val = lua_tostring(lua_state, -1);`
	`85`	`+ char const *const val = lua_tostring(lua_state, -1);`
`86`	`86`
`87`	`87`	`if (!val) {`
`88`	`88`	`throw std::runtime_error{`
Original file line number	Diff line number	Diff line change
`@@ -54,7 +54,7 @@ struct table_sql {`
`54`	`54`
`55`	`55`	`struct middle_pgsql_t : public slim_middle_t`
`56`	`56`	`{`
`57`		`- middle_pgsql_t(options_t const *options);`
	`57`	`+ explicit middle_pgsql_t(options_t const *options);`
`58`	`58`
`59`	`59`	`void start() override;`
`60`	`60`	`void stop(thread_pool_t &pool) override;`
`@@ -79,7 +79,7 @@ struct middle_pgsql_t : public slim_middle_t`
`79`	`79`	`class table_desc`
`80`	`80`	`{`
`81`	`81`	`public:`
`82`		`- table_desc() {}`
	`82`	`+ table_desc() = default;`
`83`	`83`	`table_desc(options_t const &options, table_sql const &ts);`
`84`	`84`
`85`	`85`	`char const *name() const { return m_copy_target->name.c_str(); }`