Skip to content

Commit 207f577

Browse files
committed
Remove possible dangling pointer from node_persistent_cache
Storing a pointer to a string that might go away isn't a good idea. Using std::string instead. Also slightly improved logging.
1 parent d1766f1 commit 207f577

File tree

2 files changed

+16
-14
lines changed

2 files changed

+16
-14
lines changed

src/node-persistent-cache.cpp

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
#include "node-persistent-cache.hpp"
1414

1515
#include <cassert>
16+
#include <cstring>
17+
#include <stdexcept>
1618

1719
void node_persistent_cache::set(osmid_t id, osmium::Location location)
1820
{
@@ -25,20 +27,19 @@ osmium::Location node_persistent_cache::get(osmid_t id) const noexcept
2527
static_cast<osmium::unsigned_object_id_type>(id));
2628
}
2729

28-
node_persistent_cache::node_persistent_cache(std::string const &file_name, bool remove_file)
30+
node_persistent_cache::node_persistent_cache(std::string file_name,
31+
bool remove_file)
32+
: m_file_name(std::move(file_name)), m_remove_file(remove_file)
2933
{
30-
assert(!file_name.empty());
34+
assert(!m_file_name.empty());
3135

32-
m_fname = file_name.c_str();
33-
m_remove_file = remove_file;
34-
log_debug("Mid: loading persistent node cache from {}", m_fname);
36+
log_debug("Loading persistent node cache from '{}'.", m_file_name);
3537

3638
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg, hicpp-signed-bitwise)
37-
m_fd = open(m_fname, O_RDWR | O_CREAT, 0644);
39+
m_fd = open(m_file_name.c_str(), O_RDWR | O_CREAT, 0644);
3840
if (m_fd < 0) {
39-
log_error("Cannot open location cache file '{}': {}", m_fname,
40-
std::strerror(errno));
41-
throw std::runtime_error{"Unable to open flatnode file."};
41+
throw std::runtime_error{"Unable to open flatnode file '{}': {}"_format(
42+
m_file_name, std::strerror(errno))};
4243
}
4344

4445
m_index.reset(new index_t{m_fd});
@@ -53,9 +54,9 @@ node_persistent_cache::~node_persistent_cache() noexcept
5354

5455
if (m_remove_file) {
5556
try {
56-
log_debug("Mid: removing persistent node cache at {}", m_fname);
57+
log_debug("Removing persistent node cache at '{}'.", m_file_name);
5758
} catch (...) {
5859
}
59-
unlink(m_fname);
60+
unlink(m_file_name.c_str());
6061
}
6162
}

src/node-persistent-cache.hpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
*/
1212

1313
#include <memory>
14+
#include <string>
1415

1516
#include <osmium/index/map/dense_file_array.hpp>
1617
#include <osmium/osm/location.hpp>
@@ -20,7 +21,7 @@
2021
class node_persistent_cache
2122
{
2223
public:
23-
node_persistent_cache(std::string const &file_name, bool remove_file);
24+
node_persistent_cache(std::string file_name, bool remove_file);
2425
~node_persistent_cache() noexcept;
2526

2627
void set(osmid_t id, osmium::Location location);
@@ -31,10 +32,10 @@ class node_persistent_cache
3132
osmium::index::map::DenseFileArray<osmium::unsigned_object_id_type,
3233
osmium::Location>;
3334

35+
std::string m_file_name;
3436
int m_fd = -1;
3537
std::unique_ptr<index_t> m_index;
36-
bool m_remove_file = false;
37-
char const *m_fname = nullptr;
38+
bool m_remove_file;
3839
};
3940

4041
#endif // OSM2PGSQL_NODE_PERSISTENT_CACHE_HPP

0 commit comments

Comments
 (0)