Skip to content

Commit e2b6d1a

Browse files
committed
Various code cleanups
* Fix typos * Modernize C++ * Fix clang-tidy warnings * Formatting cleanups
1 parent 7892613 commit e2b6d1a

22 files changed

Lines changed: 126 additions & 129 deletions

src/db-copy-mgr.hpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ template <typename DELETER>
1313
class db_copy_mgr_t
1414
{
1515
public:
16-
db_copy_mgr_t(std::shared_ptr<db_copy_thread_t> const &processor)
16+
explicit db_copy_mgr_t(std::shared_ptr<db_copy_thread_t> const &processor)
1717
: m_processor(processor)
1818
{}
1919

@@ -49,7 +49,7 @@ class db_copy_mgr_t
4949

5050
// Expect that a column has been written last which ended in a '\t'.
5151
// Replace it with the row delimiter '\n'.
52-
auto sz = buf.size();
52+
auto const sz = buf.size();
5353
assert(buf[sz - 1] == '\t');
5454
buf[sz - 1] = '\n';
5555

@@ -67,7 +67,7 @@ class db_copy_mgr_t
6767
void add_columns(T value, ARGS &&... args)
6868
{
6969
add_column(value);
70-
add_columns(args...);
70+
add_columns(std::forward<ARGS>(args)...);
7171
}
7272

7373
template <typename T>
@@ -79,7 +79,7 @@ class db_copy_mgr_t
7979
/**
8080
* Add a column entry of simple type.
8181
*
82-
* Writes the column with the escaping apporpriate for the type and
82+
* Writes the column with the escaping appropriate for the type and
8383
* a column delimiter.
8484
*/
8585
template <typename T>
@@ -136,7 +136,7 @@ class db_copy_mgr_t
136136
*/
137137
void finish_array()
138138
{
139-
auto idx = m_current->buffer.size() - 1;
139+
auto const idx = m_current->buffer.size() - 1;
140140
if (m_current->buffer[idx] == '{')
141141
m_current->buffer += '}';
142142
else
@@ -222,7 +222,7 @@ class db_copy_mgr_t
222222
*/
223223
void finish_hash()
224224
{
225-
auto idx = m_current->buffer.size() - 1;
225+
auto const idx = m_current->buffer.size() - 1;
226226
if (!m_current->buffer.empty() && m_current->buffer[idx] == ',') {
227227
m_current->buffer[idx] = '\t';
228228
} else {
@@ -237,7 +237,7 @@ class db_copy_mgr_t
237237
*/
238238
void add_hex_geom(std::string const &wkb)
239239
{
240-
char const *lookup_hex = "0123456789ABCDEF";
240+
char const *const lookup_hex = "0123456789ABCDEF";
241241

242242
for (char c : wkb) {
243243
m_current->buffer += lookup_hex[(c >> 4) & 0xf];

src/db-copy.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ struct db_cmd_finish_t : public db_cmd_t
225225
class db_copy_thread_t
226226
{
227227
public:
228-
db_copy_thread_t(std::string const &conninfo);
228+
explicit db_copy_thread_t(std::string const &conninfo);
229229

230230
db_copy_thread_t(db_copy_thread_t const &) = delete;
231231
db_copy_thread_t &operator=(db_copy_thread_t const &) = delete;

src/domain-matcher.hpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
#include <cstring>
77
#include <string>
88

9-
109
/**
1110
* Returns the tag specific name, if applicable.
1211
*

src/expire-tiles.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,7 @@ void expire_tiles::merge_and_destroy(expire_tiles &other)
452452
tile_width, other.tile_width)};
453453
}
454454

455-
if (m_dirty_tiles.size() == 0) {
455+
if (m_dirty_tiles.empty()) {
456456
m_dirty_tiles = std::move(other.m_dirty_tiles);
457457
} else {
458458
m_dirty_tiles.insert(other.m_dirty_tiles.begin(),

src/gazetteer-style.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ enum : int
1717
MAX_ADMINLEVEL = 15
1818
};
1919

20-
}
20+
} // anonymous namespace
2121

2222
namespace pt = boost::property_tree;
2323

src/id-tracker.cpp

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,17 @@ namespace {
2020
*/
2121
struct block
2222
{
23-
block() : bits(BLOCK_SIZE >> 5, 0) {}
23+
block() : bits(BLOCK_SIZE >> 5U, 0) {}
2424
inline bool operator[](size_t i) const
2525
{
26-
return (bits[i >> 5] & (1 << (i & 0x1f))) > 0;
26+
return (bits[i >> 5U] & (1U << (i & 0x1fU))) > 0;
2727
}
2828
//returns true if the value actually caused a bit to flip
2929
inline bool set(size_t i, bool value)
3030
{
31-
uint32_t &bit = bits[i >> 5];
31+
uint32_t &bit = bits[i >> 5U];
3232
uint32_t old = bit;
33-
uint32_t mask = 1 << (i & 0x1f);
33+
uint32_t mask = 1U << (i & 0x1fU);
3434
//allow the bit to become 1 if not already
3535
if (value) {
3636
bit |= mask;
@@ -48,20 +48,20 @@ struct block
4848
// returns BLOCK_SIZE if a set bit isn't found
4949
size_t next_set(size_t start) const
5050
{
51-
uint32_t bit_i = start >> 5;
51+
uint32_t bit_i = start >> 5U;
5252

53-
while ((bit_i < (BLOCK_SIZE >> 5)) && (bits[bit_i] == 0)) {
53+
while ((bit_i < (BLOCK_SIZE >> 5U)) && (bits[bit_i] == 0)) {
5454
++bit_i;
5555
}
5656

57-
if (bit_i >= (BLOCK_SIZE >> 5)) {
57+
if (bit_i >= (BLOCK_SIZE >> 5U)) {
5858
return BLOCK_SIZE;
5959
}
6060
uint32_t bit = bits[bit_i];
61-
size_t idx = bit_i << 5;
62-
while ((bit & 1) == 0) {
61+
size_t idx = bit_i << 5U;
62+
while ((bit & 1U) == 0) {
6363
++idx;
64-
bit >>= 1;
64+
bit >>= 1U;
6565
}
6666
return idx;
6767
}
@@ -80,7 +80,7 @@ struct id_tracker::pimpl
8080
bool set(osmid_t id, bool value);
8181
osmid_t pop_min();
8282

83-
typedef std::map<osmid_t, block> map_t;
83+
using map_t = std::map<osmid_t, block>;
8484
map_t pending;
8585
osmid_t old_id;
8686
size_t count;
@@ -92,21 +92,22 @@ struct id_tracker::pimpl
9292

9393
bool id_tracker::pimpl::get(osmid_t id) const
9494
{
95-
const osmid_t block = id >> BLOCK_BITS, offset = id & BLOCK_MASK;
96-
map_t::const_iterator itr = pending.find(block);
97-
bool result = false;
95+
osmid_t const block = id >> BLOCK_BITS;
96+
osmid_t const offset = id & BLOCK_MASK;
97+
auto const itr = pending.find(block);
9898

99-
if (itr != pending.end()) {
100-
result = itr->second[offset];
99+
if (itr == pending.end()) {
100+
return false;
101101
}
102102

103-
return result;
103+
return itr->second[offset];
104104
}
105105

106106
bool id_tracker::pimpl::set(osmid_t id, bool value)
107107
{
108-
const osmid_t block = id >> BLOCK_BITS, offset = id & BLOCK_MASK;
109-
bool flipped = pending[block].set(offset, value);
108+
osmid_t const block = id >> BLOCK_BITS;
109+
osmid_t const offset = id & BLOCK_MASK;
110+
bool const flipped = pending[block].set(offset, value);
110111
// a set may potentially invalidate a next_start, as the bit
111112
// set might be before the position of next_start.
112113
if (next_start) {

src/middle-pgsql.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -565,8 +565,8 @@ void middle_pgsql_t::relations_set(osmium::Relation const &rel)
565565

566566
// parts
567567
m_db_copy.new_array();
568-
for (int i = 0; i < 3; ++i) {
569-
for (auto it : parts[i]) {
568+
for (auto const &part : parts) {
569+
for (auto it : part) {
570570
m_db_copy.add_array_elem(it);
571571
}
572572
}
@@ -813,9 +813,9 @@ middle_pgsql_t::get_query_instance(std::shared_ptr<middle_t> const &from) const
813813

814814
// NOTE: this is thread safe for use in pending async processing only because
815815
// during that process they are only read from
816-
std::unique_ptr<middle_query_pgsql_t> mid(new middle_query_pgsql_t(
817-
src->out_options->database_options.conninfo().c_str(), src->cache,
818-
src->persistent_cache));
816+
std::unique_ptr<middle_query_pgsql_t> mid(new middle_query_pgsql_t{
817+
src->out_options->database_options.conninfo(), src->cache,
818+
src->persistent_cache});
819819

820820
// We use a connection per table to enable the use of COPY
821821
for (int i = 0; i < NUM_TABLES; ++i) {

src/options.cpp

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -202,8 +202,9 @@ void long_usage(char *arg0, bool verbose = false)
202202
pbf - OSM binary format.\n\
203203
-O|--output Output backend.\n\
204204
pgsql - Output to a PostGIS database (default)\n\
205+
flex - More flexible output to PostGIS database\n\
205206
multi - Multiple Custom Table Output to a PostGIS \n\
206-
database (requires style file for configuration)\n\
207+
database (deprecated)\n\
207208
gazetteer - Output to a PostGIS database for Nominatim\n\
208209
null - No output. Useful for testing. Still creates tables if --slim is specified.\n");
209210
#ifdef HAVE_LUA
@@ -345,7 +346,7 @@ options_t::options_t(int argc, char *argv[]) : options_t()
345346
#ifdef HAVE_GENERIC_PROJ
346347
projection = reprojection::create_projection(atoi(optarg));
347348
#else
348-
throw std::runtime_error("Generic projections not available.");
349+
throw std::runtime_error{"Generic projections not available."};
349350
#endif
350351
break;
351352
case 'p':
@@ -389,18 +390,18 @@ options_t::options_t(int argc, char *argv[]) : options_t()
389390
break;
390391
case 'e':
391392
if (!optarg || optarg[0] == '-') {
392-
throw std::runtime_error(
393+
throw std::runtime_error{
393394
"Missing argument for option --expire-tiles. Zoom "
394-
"levels must be positive.\n");
395+
"levels must be positive.\n"};
395396
}
396397
char *next_char;
397398
expire_tiles_zoom_min =
398399
static_cast<uint32_t>(std::strtoul(optarg, &next_char, 10));
399400
if (expire_tiles_zoom_min == 0) {
400-
throw std::runtime_error(
401+
throw std::runtime_error{
401402
"Bad argument for option --expire-tiles. "
402403
"Minimum zoom level must be larger "
403-
"than 0.\n");
404+
"than 0.\n"};
404405
}
405406
// The first character after the number is ignored because that is the separating hyphen.
406407
if (*next_char == '-') {
@@ -411,20 +412,20 @@ options_t::options_t(int argc, char *argv[]) : options_t()
411412
expire_tiles_zoom = static_cast<uint32_t>(
412413
std::strtoul(next_char, &after_maxzoom, 10));
413414
if (expire_tiles_zoom == 0 || *after_maxzoom != '\0') {
414-
throw std::runtime_error("Invalid maximum zoom level "
415-
"given for tile expiry.\n");
415+
throw std::runtime_error{"Invalid maximum zoom level "
416+
"given for tile expiry.\n"};
416417
}
417418
} else {
418-
throw std::runtime_error(
419-
"Invalid maximum zoom level given for tile expiry.\n");
419+
throw std::runtime_error{
420+
"Invalid maximum zoom level given for tile expiry.\n"};
420421
}
421422
} else if (*next_char == '\0') {
422423
// end of string, no second zoom level given
423424
expire_tiles_zoom = expire_tiles_zoom_min;
424425
} else {
425-
throw std::runtime_error("Minimum and maximum zoom level for "
426+
throw std::runtime_error{"Minimum and maximum zoom level for "
426427
"tile expiry must be separated by "
427-
"'-'.\n");
428+
"'-'.\n"};
428429
}
429430
break;
430431
case 'o':
@@ -446,8 +447,8 @@ options_t::options_t(int argc, char *argv[]) : options_t()
446447
break;
447448
case 'k':
448449
if (hstore_mode != HSTORE_NONE) {
449-
throw std::runtime_error("You can not specify both --hstore "
450-
"(-k) and --hstore-all (-j)\n");
450+
throw std::runtime_error{"You can not specify both --hstore "
451+
"(-k) and --hstore-all (-j)\n"};
451452
}
452453
hstore_mode = HSTORE_NORM;
453454
break;
@@ -456,13 +457,13 @@ options_t::options_t(int argc, char *argv[]) : options_t()
456457
break;
457458
case 'j':
458459
if (hstore_mode != HSTORE_NONE) {
459-
throw std::runtime_error("You can not specify both --hstore "
460-
"(-k) and --hstore-all (-j)\n");
460+
throw std::runtime_error{"You can not specify both --hstore "
461+
"(-k) and --hstore-all (-j)\n"};
461462
}
462463
hstore_mode = HSTORE_ALL;
463464
break;
464465
case 'z':
465-
hstore_columns.push_back(optarg);
466+
hstore_columns.emplace_back(optarg);
466467
break;
467468
case 'G':
468469
enable_multi = true;
@@ -477,13 +478,13 @@ options_t::options_t(int argc, char *argv[]) : options_t()
477478
parallel_indexing = false;
478479
break;
479480
case 204:
480-
if (strcmp(optarg, "dense") == 0) {
481+
if (std::strcmp(optarg, "dense") == 0) {
481482
alloc_chunkwise = ALLOC_DENSE;
482-
} else if (strcmp(optarg, "chunk") == 0) {
483+
} else if (std::strcmp(optarg, "chunk") == 0) {
483484
alloc_chunkwise = ALLOC_DENSE | ALLOC_DENSE_CHUNK;
484-
} else if (strcmp(optarg, "sparse") == 0) {
485+
} else if (std::strcmp(optarg, "sparse") == 0) {
485486
alloc_chunkwise = ALLOC_SPARSE;
486-
} else if (strcmp(optarg, "optimized") == 0) {
487+
} else if (std::strcmp(optarg, "optimized") == 0) {
487488
alloc_chunkwise = ALLOC_DENSE | ALLOC_SPARSE;
488489
} else {
489490
throw std::runtime_error{
@@ -543,7 +544,7 @@ options_t::options_t(int argc, char *argv[]) : options_t()
543544

544545
//get the input files
545546
while (optind < argc) {
546-
input_files.push_back(std::string(argv[optind]));
547+
input_files.emplace_back(argv[optind]);
547548
optind++;
548549
}
549550

@@ -566,19 +567,19 @@ options_t::options_t(int argc, char *argv[]) : options_t()
566567
void options_t::check_options()
567568
{
568569
if (append && create) {
569-
throw std::runtime_error("--append and --create options can not be "
570-
"used at the same time!\n");
570+
throw std::runtime_error{"--append and --create options can not be "
571+
"used at the same time!\n"};
571572
}
572573

573574
if (append && !slim) {
574-
throw std::runtime_error("--append can only be used with slim mode!\n");
575+
throw std::runtime_error{"--append can only be used with slim mode!\n"};
575576
}
576577

577578
if (droptemp && !slim) {
578-
throw std::runtime_error("--drop only makes sense with --slim.\n");
579+
throw std::runtime_error{"--drop only makes sense with --slim.\n"};
579580
}
580581

581-
if (hstore_mode == HSTORE_NONE && hstore_columns.size() == 0 &&
582+
if (hstore_mode == HSTORE_NONE && hstore_columns.empty() &&
582583
hstore_match_only) {
583584
fprintf(stderr,
584585
"Warning: --hstore-match-only only makes sense with --hstore, "
@@ -587,7 +588,7 @@ void options_t::check_options()
587588
}
588589

589590
if (enable_hstore_index && hstore_mode == HSTORE_NONE &&
590-
hstore_columns.size() == 0) {
591+
hstore_columns.empty()) {
591592
fprintf(stderr, "Warning: --hstore-add-index only makes sense with "
592593
"hstore enabled.\n");
593594
enable_hstore_index = false;
@@ -601,8 +602,8 @@ void options_t::check_options()
601602

602603
if (cache == 0) {
603604
if (!slim) {
604-
throw std::runtime_error(
605-
"Ram node cache can only be disable in slim mode.\n");
605+
throw std::runtime_error{
606+
"Ram node cache can only be disable in slim mode.\n"};
606607
}
607608
if (!flat_node_cache_enabled) {
608609
fprintf(stderr, "WARNING: ram cache is disabled. This will likely "

src/osm2pgsql.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ int main(int argc, char *argv[])
8787
*/
8888
parse_stats_t stats;
8989
//read in the input files one by one
90-
for (auto const filename : options.input_files) {
90+
for (auto const &filename : options.input_files) {
9191
//read the actual input
9292
fprintf(stderr, "\nReading in file: %s\n", filename.c_str());
9393
time_t start = time(nullptr);

0 commit comments

Comments
 (0)