diff --git a/CHANGELOG.md b/CHANGELOG.md index 108e7303..b6f5ec49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,71 @@ +# 2.81.0 + +* Add `--drop-by-attribute-as-needed=`*attribute* to drop the features with + the lowest values of a numeric attribute from oversized tiles, and + `--drop-by-attribute-order=desc` to drop the highest values instead. + Features exactly at the threshold are kept rather than dropped. (#384, #385) +* Add `--exclude-all-tile-geometries` to tile-join, to produce tiles that + carry only attributes. (#382) +* Generate each tool's usage message from the same option table that + `getopt_long()` reads, so the hand-written lists in tile-join, + tippecanoe-overzoom, tippecanoe-json-tool, tippecanoe-decode, and + tippecanoe-enumerate can no longer fall behind the options actually + accepted. Options previously reachable only by their short names are now + listed. tippecanoe-overzoom reports a missing `-o` instead of passing a + null pointer to `fopen()`, and tippecanoe prints its usage when run with + no arguments. (#409) +* Fix the radix sort used by `--prefer-radix-sort`. A bucket written out + directly rather than through the merge was written one byte longer than + its length prefix claimed, desynchronizing everything read from the + geometry after it. Subdividing could also recurse forever once it ran out + of files to split with, shifting by the full width of the index and + writing past the end of the arrays of buckets. Radix-sorted output is now + checked against the in-memory sort rather than against a stored copy. (#404) +* Read FlatGeobuf integer and float properties as numbers. They were tagged + with types that the rest of tippecanoe does not treat as numeric, so they + were reported in tilestats as "mixed", with quoted values and no min or + max, and warned when used as a feature ID. ULong properties are now also + read as unsigned rather than signed. (#395) +* Respect the `-t` temporary directory option in sorting operations, which + previously always used the system temporary directory. (#368) +* Keep `--generate-variable-depth-tile-pyramid` from silently dropping + features whose explicit per-feature `minzoom` is deeper than the zoom at + which their region becomes a leaf. Such a feature was excluded from the + leaf tile while its children were never generated, so it appeared at no + zoom at all. (#397, #399) +* Drop a polygon hole that no remaining ring can parent, instead of failing + the whole run. Degenerate input could abort tiling over a single + unrepresentable sliver. (#401) +* Clamp the feature extent to the `long long` range before converting it, + at both ends. The previous `extent <= LLONG_MAX` guard was doubly wrong: + `LLONG_MAX` is not representable as a double and rounds up, so an extent + at the very top of the range overflowed the conversion and came out as the + most negative value rather than the largest, and the guard admitted + everything below `LLONG_MIN` as well, which overflowed the other way. Areas + are signed, so holes that outweigh their rings can reach the low end. (#406) +* Initialize the full width of the `mvt_value` numeric union, which left the + bytes of the wider unused member indeterminate even though the implicit + copy constructor copies the union as a whole. (#406) +* Replace all variable-length arrays with `std::vector` and `std::string`, + and build with `-Wvla`. VLAs are a compiler extension rather than standard + C++, and clang warns about every one of them by default. (#406) +* Remove the unused Dockerfiles, Travis configuration, and lambda + directory. (#365) +* Correct README statements that disagreed with the code. Among them, `-aD` + and `-aS` were documented the wrong way round, + `--limit-base-zoom-to-maximum-zoom` was given as `-Pb` rather than `-pb`, + and the dot-dropping description had both the fraction and the zoom + direction backwards: tippecanoe keeps 1/2.5 of the dots at zooms below the + base zoom, rather than dropping that share above it. (#410) +* Generate `man/tippecanoe.1` with go-md2man rather than md2man-roff, which + is packaged only as a Ruby gem and so had let the page drift out of date. + The page now has a proper header and a NAME section, so `man -k` and + `whatis` can find it, and no longer silently drops or mangles text the old + converter mishandled. CI checks it against README.md. (#408) +* Documentation fixes: correct three misspellings in the README and man + page, repair the dead All Streets link, and tag more README code blocks + with their language. (#375, #391, #400) + # 2.80.0 * Remove undocumented command-line options diff --git a/Makefile b/Makefile index df38c0d6..84ad090c 100644 --- a/Makefile +++ b/Makefile @@ -11,7 +11,7 @@ CXX := $(CXX) CFLAGS := $(CFLAGS) -fPIE -DBUILD_INFO=$(BUILD_INFO) CXXFLAGS := $(CXXFLAGS) -std=c++17 -fPIE -DBUILD_INFO=$(BUILD_INFO) LDFLAGS := $(LDFLAGS) -WARNING_FLAGS := -Wall -Wshadow -Wsign-compare -Wextra -Wunreachable-code -Wuninitialized -Wshadow +WARNING_FLAGS := -Wall -Wshadow -Wsign-compare -Wextra -Wunreachable-code -Wuninitialized -Wshadow -Wvla RELEASE_FLAGS := -O3 -DNDEBUG DEBUG_FLAGS := -O0 -DDEBUG -fno-inline-functions -fno-omit-frame-pointer diff --git a/main.cpp b/main.cpp index 5842f988..e084e6e1 100644 --- a/main.cpp +++ b/main.cpp @@ -215,7 +215,7 @@ void init_cpus() { // MacOS can run out of system file descriptors // even if we stay under the rlimit, so try to // find out the real limit. - long long fds[MAX_FILES]; + std::vector fds(MAX_FILES); long long i; for (i = 0; i < MAX_FILES; i++) { fds[i] = open(get_null_device(), O_RDONLY | O_CLOEXEC); @@ -449,7 +449,7 @@ void *run_sort(void *v) { } void do_read_parallel(char *map, long long len, long long initial_offset, const char *reading, std::vector *readers, std::atomic *progress_seq, std::set *exclude, std::set *include, int exclude_all, int basezoom, int source, std::vector > *layermaps, int *initialized, unsigned *initial_x, unsigned *initial_y, int maxzoom, std::string layername, bool uses_gamma, std::unordered_map const *attribute_types, int separator, double *dist_sum, size_t *dist_count, double *area_sum, bool want_dist, bool filters) { - long long segs[CPUS + 1]; + std::vector segs(CPUS + 1); segs[0] = 0; segs[CPUS] = len; @@ -461,11 +461,11 @@ void do_read_parallel(char *map, long long len, long long initial_offset, const } } - double dist_sums[CPUS]; - size_t dist_counts[CPUS]; - double area_sums[CPUS]; + std::vector dist_sums(CPUS); + std::vector dist_counts(CPUS); + std::vector area_sums(CPUS); - std::atomic layer_seq[CPUS]; + std::vector > layer_seq(CPUS); for (size_t i = 0; i < CPUS; i++) { // To preserve feature ordering, unique id for each segment // begins with that segment's offset into the input @@ -479,7 +479,7 @@ void do_read_parallel(char *map, long long len, long long initial_offset, const std::vector sst; sst.resize(CPUS); - pthread_t pthreads[CPUS]; + std::vector pthreads(CPUS); std::vector > file_subkeys; for (size_t i = 0; i < CPUS; i++) { @@ -759,47 +759,45 @@ void radix1(int *geomfds_in, int *indexfds_in, int inputs, int prefix, int split } splits = 1 << splitbits; - FILE *geomfiles[splits]; - FILE *indexfiles[splits]; - int geomfds[splits]; - int indexfds[splits]; - std::atomic sub_geompos[splits]; + std::vector geomfiles(splits); + std::vector indexfiles(splits); + std::vector geomfds(splits); + std::vector indexfds(splits); + std::vector > sub_geompos(splits); int i; for (i = 0; i < splits; i++) { sub_geompos[i] = 0; - char geomname[strlen(tmpdir) + strlen("/geom.XXXXXXXX") + 1]; - snprintf(geomname, sizeof(geomname), "%s%s", tmpdir, "/geom.XXXXXXXX"); - char indexname[strlen(tmpdir) + strlen("/index.XXXXXXXX") + 1]; - snprintf(indexname, sizeof(indexname), "%s%s", tmpdir, "/index.XXXXXXXX"); + std::string geomname = std::string(tmpdir) + "/geom.XXXXXXXX"; + std::string indexname = std::string(tmpdir) + "/index.XXXXXXXX"; - geomfds[i] = mkstemp_cloexec(geomname); + geomfds[i] = mkstemp_cloexec(&geomname[0]); if (geomfds[i] < 0) { - perror(geomname); + perror(geomname.c_str()); exit(EXIT_OPEN); } - indexfds[i] = mkstemp_cloexec(indexname); + indexfds[i] = mkstemp_cloexec(&indexname[0]); if (indexfds[i] < 0) { - perror(indexname); + perror(indexname.c_str()); exit(EXIT_OPEN); } - geomfiles[i] = fopen_oflag(geomname, "wb", O_WRONLY | O_CLOEXEC); + geomfiles[i] = fopen_oflag(geomname.c_str(), "wb", O_WRONLY | O_CLOEXEC); if (geomfiles[i] == NULL) { - perror(geomname); + perror(geomname.c_str()); exit(EXIT_OPEN); } - indexfiles[i] = fopen_oflag(indexname, "wb", O_WRONLY | O_CLOEXEC); + indexfiles[i] = fopen_oflag(indexname.c_str(), "wb", O_WRONLY | O_CLOEXEC); if (indexfiles[i] == NULL) { - perror(indexname); + perror(indexname.c_str()); exit(EXIT_OPEN); } *availfiles -= 4; - unlink(geomname); - unlink(indexname); + unlink(geomname.c_str()); + unlink(indexname.c_str()); } for (i = 0; i < inputs; i++) { @@ -928,13 +926,13 @@ void radix1(int *geomfds_in, int *indexfds_in, int inputs, int prefix, int split } size_t nmerges = (indexpos + unit - 1) / unit; - struct mergelist merges[nmerges]; + std::vector merges(nmerges); for (size_t a = 0; a < nmerges; a++) { merges[a].start = merges[a].end = 0; } - pthread_t pthreads[CPUS]; + std::vector pthreads(CPUS); std::vector args; for (size_t a = 0; a < CPUS; a++) { @@ -942,7 +940,7 @@ void radix1(int *geomfds_in, int *indexfds_in, int inputs, int prefix, int split a, CPUS, indexpos, - merges, + merges.data(), indexfds[i], nmerges, unit, @@ -980,7 +978,7 @@ void radix1(int *geomfds_in, int *indexfds_in, int inputs, int prefix, int split madvise(geommap, geomst.st_size, MADV_RANDOM); madvise(geommap, geomst.st_size, MADV_WILLNEED); - merge(merges, nmerges, (unsigned char *) indexmap, indexfile, bytes, geommap, geomfile, geompos_out, progress, progress_max, progress_reported, maxzoom, gamma, ds); + merge(merges.data(), nmerges, (unsigned char *) indexmap, indexfile, bytes, geommap, geomfile, geompos_out, progress, progress_max, progress_reported, maxzoom, gamma, ds); madvise(indexmap, indexst.st_size, MADV_DONTNEED); if (munmap(indexmap, indexst.st_size) < 0) { @@ -1119,8 +1117,8 @@ void radix(std::vector &readers, int nreaders, FILE *geomfile, FI mem /= 2; long long geom_total = 0; - int geomfds[nreaders]; - int indexfds[nreaders]; + std::vector geomfds(nreaders); + std::vector indexfds(nreaders); for (int i = 0; i < nreaders; i++) { geomfds[i] = readers[i].geomfd; indexfds[i] = readers[i].indexfd; @@ -1133,12 +1131,12 @@ void radix(std::vector &readers, int nreaders, FILE *geomfile, FI geom_total += geomst.st_size; } - struct drop_state ds[maxzoom + 1]; - prep_drop_states(ds, maxzoom, basezoom, droprate); + std::vector ds(maxzoom + 1); + prep_drop_states(ds.data(), maxzoom, basezoom, droprate); long long progress = 0, progress_max = geom_total, progress_reported = -1; long long availfiles_before = availfiles; - radix1(geomfds, indexfds, nreaders, 0, splits, mem, tmpdir, &availfiles, geomfile, indexfile, geompos, &progress, &progress_max, &progress_reported, maxzoom, basezoom, droprate, gamma, ds); + radix1(geomfds.data(), indexfds.data(), nreaders, 0, splits, mem, tmpdir, &availfiles, geomfile, indexfile, geompos, &progress, &progress_max, &progress_reported, maxzoom, basezoom, droprate, gamma, ds.data()); if (availfiles - 2 * nreaders != availfiles_before) { fprintf(stderr, "Internal error: miscounted available file descriptors: %lld vs %lld\n", availfiles - 2 * nreaders, availfiles); @@ -1247,79 +1245,72 @@ std::pair read_input(std::vector &sources, char *fname, i for (size_t i = 0; i < CPUS; i++) { struct reader *r = &readers[i]; - char poolname[strlen(tmpdir) + strlen("/pool.XXXXXXXX") + 1]; - char treename[strlen(tmpdir) + strlen("/tree.XXXXXXXX") + 1]; - char geomname[strlen(tmpdir) + strlen("/geom.XXXXXXXX") + 1]; - char indexname[strlen(tmpdir) + strlen("/index.XXXXXXXX") + 1]; - char vertexname[strlen(tmpdir) + strlen("/vertex.XXXXXXXX") + 1]; - char nodename[strlen(tmpdir) + strlen("/node.XXXXXXXX") + 1]; + std::string poolname = std::string(tmpdir) + "/pool.XXXXXXXX"; + std::string treename = std::string(tmpdir) + "/tree.XXXXXXXX"; + std::string geomname = std::string(tmpdir) + "/geom.XXXXXXXX"; + std::string indexname = std::string(tmpdir) + "/index.XXXXXXXX"; + std::string vertexname = std::string(tmpdir) + "/vertex.XXXXXXXX"; + std::string nodename = std::string(tmpdir) + "/node.XXXXXXXX"; - snprintf(poolname, sizeof(poolname), "%s%s", tmpdir, "/pool.XXXXXXXX"); - snprintf(treename, sizeof(treename), "%s%s", tmpdir, "/tree.XXXXXXXX"); - snprintf(geomname, sizeof(geomname), "%s%s", tmpdir, "/geom.XXXXXXXX"); - snprintf(indexname, sizeof(indexname), "%s%s", tmpdir, "/index.XXXXXXXX"); - snprintf(vertexname, sizeof(vertexname), "%s%s", tmpdir, "/vertex.XXXXXXXX"); - snprintf(nodename, sizeof(nodename), "%s%s", tmpdir, "/node.XXXXXXXX"); - - r->poolfd = mkstemp_cloexec(poolname); + r->poolfd = mkstemp_cloexec(&poolname[0]); if (r->poolfd < 0) { - perror(poolname); + perror(poolname.c_str()); exit(EXIT_OPEN); } - r->treefd = mkstemp_cloexec(treename); + r->treefd = mkstemp_cloexec(&treename[0]); if (r->treefd < 0) { - perror(treename); + perror(treename.c_str()); exit(EXIT_OPEN); } - r->geomfd = mkstemp_cloexec(geomname); + r->geomfd = mkstemp_cloexec(&geomname[0]); if (r->geomfd < 0) { - perror(geomname); + perror(geomname.c_str()); exit(EXIT_OPEN); } - r->indexfd = mkstemp_cloexec(indexname); + r->indexfd = mkstemp_cloexec(&indexname[0]); if (r->indexfd < 0) { - perror(indexname); + perror(indexname.c_str()); exit(EXIT_OPEN); } - r->vertexfd = mkstemp_cloexec(vertexname); + r->vertexfd = mkstemp_cloexec(&vertexname[0]); if (r->vertexfd < 0) { - perror(vertexname); + perror(vertexname.c_str()); exit(EXIT_OPEN); } - r->nodefd = mkstemp_cloexec(nodename); + r->nodefd = mkstemp_cloexec(&nodename[0]); if (r->nodefd < 0) { - perror(nodename); + perror(nodename.c_str()); exit(EXIT_OPEN); } r->poolfile = memfile_open(r->poolfd); if (r->poolfile == NULL) { - perror(poolname); + perror(poolname.c_str()); exit(EXIT_OPEN); } r->treefile = memfile_open(r->treefd); if (r->treefile == NULL) { - perror(treename); + perror(treename.c_str()); exit(EXIT_OPEN); } - r->geomfile = fopen_oflag(geomname, "wb", O_WRONLY | O_CLOEXEC); + r->geomfile = fopen_oflag(geomname.c_str(), "wb", O_WRONLY | O_CLOEXEC); if (r->geomfile == NULL) { - perror(geomname); + perror(geomname.c_str()); exit(EXIT_OPEN); } - r->indexfile = fopen_oflag(indexname, "wb", O_WRONLY | O_CLOEXEC); + r->indexfile = fopen_oflag(indexname.c_str(), "wb", O_WRONLY | O_CLOEXEC); if (r->indexfile == NULL) { - perror(indexname); + perror(indexname.c_str()); exit(EXIT_OPEN); } - r->vertexfile = fopen_oflag(vertexname, "w+b", O_RDWR | O_CLOEXEC); + r->vertexfile = fopen_oflag(vertexname.c_str(), "w+b", O_RDWR | O_CLOEXEC); if (r->vertexfile == NULL) { - perror(("open vertexfile " + std::string(vertexname)).c_str()); + perror(("open vertexfile " + vertexname).c_str()); exit(EXIT_OPEN); } - r->nodefile = fopen_oflag(nodename, "w+b", O_RDWR | O_CLOEXEC); + r->nodefile = fopen_oflag(nodename.c_str(), "w+b", O_RDWR | O_CLOEXEC); if (r->nodefile == NULL) { - perror(nodename); + perror(nodename.c_str()); exit(EXIT_OPEN); } r->geompos = 0; @@ -1327,12 +1318,12 @@ std::pair read_input(std::vector &sources, char *fname, i r->vertexpos = 0; r->nodepos = 0; - unlink(poolname); - unlink(treename); - unlink(geomname); - unlink(indexname); - unlink(vertexname); - unlink(nodename); + unlink(poolname.c_str()); + unlink(treename.c_str()); + unlink(geomname.c_str()); + unlink(indexname.c_str()); + unlink(vertexname.c_str()); + unlink(nodename.c_str()); // To distinguish a null value { @@ -1357,8 +1348,8 @@ std::pair read_input(std::vector &sources, char *fname, i std::atomic progress_seq(0); // 2 * CPUS: One per reader thread, one per tiling thread - int initialized[2 * CPUS]; - unsigned initial_x[2 * CPUS], initial_y[2 * CPUS]; + std::vector initialized(2 * CPUS); + std::vector initial_x(2 * CPUS), initial_y(2 * CPUS); for (size_t i = 0; i < 2 * CPUS; i++) { initialized[i] = initial_x[i] = initial_y[i] = 0; } @@ -1489,10 +1480,10 @@ std::pair read_input(std::vector &sources, char *fname, i exit(EXIT_MEMORY); } - std::atomic layer_seq[CPUS]; - double dist_sums[CPUS]; - size_t dist_counts[CPUS]; - double area_sums[CPUS]; + std::vector > layer_seq(CPUS); + std::vector dist_sums(CPUS); + std::vector dist_counts(CPUS); + std::vector area_sums(CPUS); std::vector sst; sst.resize(CPUS); @@ -1562,10 +1553,10 @@ std::pair read_input(std::vector &sources, char *fname, i exit(EXIT_MEMORY); } - std::atomic layer_seq[CPUS]; - double dist_sums[CPUS]; - size_t dist_counts[CPUS]; - double area_sums[CPUS]; + std::vector > layer_seq(CPUS); + std::vector dist_sums(CPUS); + std::vector dist_counts(CPUS); + std::vector area_sums(CPUS); std::vector sst; sst.resize(CPUS); @@ -1622,10 +1613,10 @@ std::pair read_input(std::vector &sources, char *fname, i } if (sources[source].format == "csv" || (sources[source].file.size() > 4 && sources[source].file.substr(sources[source].file.size() - 4) == std::string(".csv"))) { - std::atomic layer_seq[CPUS]; - double dist_sums[CPUS]; - size_t dist_counts[CPUS]; - double area_sums[CPUS]; + std::vector > layer_seq(CPUS); + std::vector dist_sums(CPUS); + std::vector dist_counts(CPUS); + std::vector area_sums(CPUS); std::vector sst; sst.resize(CPUS); @@ -1710,7 +1701,7 @@ std::pair read_input(std::vector &sources, char *fname, i } if (map != NULL && map != MAP_FAILED && read_parallel_this) { - do_read_parallel(map, st.st_size - off, overall_offset, reading.c_str(), &readers, &progress_seq, exclude, include, exclude_all, basezoom, layer, &layermaps, initialized, initial_x, initial_y, maxzoom, sources[layer].layer, uses_gamma, attribute_types, read_parallel_this, &dist_sum, &dist_count, &area_sum, guess_maxzoom, prefilter != NULL || postfilter != NULL); + do_read_parallel(map, st.st_size - off, overall_offset, reading.c_str(), &readers, &progress_seq, exclude, include, exclude_all, basezoom, layer, &layermaps, initialized.data(), initial_x.data(), initial_y.data(), maxzoom, sources[layer].layer, uses_gamma, attribute_types, read_parallel_this, &dist_sum, &dist_count, &area_sum, guess_maxzoom, prefilter != NULL || postfilter != NULL); overall_offset += st.st_size - off; checkdisk(&readers); @@ -1742,19 +1733,18 @@ std::pair read_input(std::vector &sources, char *fname, i if (read_parallel_this) { // Serial reading of chunks that are then parsed in parallel - char readname[strlen(tmpdir) + strlen("/read.XXXXXXXX") + 1]; - snprintf(readname, sizeof(readname), "%s%s", tmpdir, "/read.XXXXXXXX"); - int readfd = mkstemp_cloexec(readname); + std::string readname = std::string(tmpdir) + "/read.XXXXXXXX"; + int readfd = mkstemp_cloexec(&readname[0]); if (readfd < 0) { - perror(readname); + perror(readname.c_str()); exit(EXIT_OPEN); } FILE *readfp = fdopen(readfd, "w"); if (readfp == NULL) { - perror(readname); + perror(readname.c_str()); exit(EXIT_OPEN); } - unlink(readname); + unlink(readname.c_str()); std::atomic is_parsing(0); long long ahead = 0; @@ -1789,25 +1779,25 @@ std::pair read_input(std::vector &sources, char *fname, i } fflush(readfp); - start_parsing(readfd, streamfpopen(readfp), initial_offset, ahead, &is_parsing, ¶llel_parser, parser_created, reading.c_str(), &readers, &progress_seq, exclude, include, exclude_all, basezoom, layer, layermaps, initialized, initial_x, initial_y, maxzoom, sources[layer].layer, gamma != 0, attribute_types, read_parallel_this, &dist_sum, &dist_count, &area_sum, guess_maxzoom, prefilter != NULL || postfilter != NULL); + start_parsing(readfd, streamfpopen(readfp), initial_offset, ahead, &is_parsing, ¶llel_parser, parser_created, reading.c_str(), &readers, &progress_seq, exclude, include, exclude_all, basezoom, layer, layermaps, initialized.data(), initial_x.data(), initial_y.data(), maxzoom, sources[layer].layer, gamma != 0, attribute_types, read_parallel_this, &dist_sum, &dist_count, &area_sum, guess_maxzoom, prefilter != NULL || postfilter != NULL); initial_offset += ahead; overall_offset += ahead; checkdisk(&readers); ahead = 0; - snprintf(readname, sizeof(readname), "%s%s", tmpdir, "/read.XXXXXXXX"); - readfd = mkstemp_cloexec(readname); + readname = std::string(tmpdir) + "/read.XXXXXXXX"; + readfd = mkstemp_cloexec(&readname[0]); if (readfd < 0) { - perror(readname); + perror(readname.c_str()); exit(EXIT_OPEN); } readfp = fdopen(readfd, "w"); if (readfp == NULL) { - perror(readname); + perror(readname.c_str()); exit(EXIT_OPEN); } - unlink(readname); + unlink(readname.c_str()); } } } @@ -1826,7 +1816,7 @@ std::pair read_input(std::vector &sources, char *fname, i fflush(readfp); if (ahead > 0) { - start_parsing(readfd, streamfpopen(readfp), initial_offset, ahead, &is_parsing, ¶llel_parser, parser_created, reading.c_str(), &readers, &progress_seq, exclude, include, exclude_all, basezoom, layer, layermaps, initialized, initial_x, initial_y, maxzoom, sources[layer].layer, gamma != 0, attribute_types, read_parallel_this, &dist_sum, &dist_count, &area_sum, guess_maxzoom, prefilter != NULL || postfilter != NULL); + start_parsing(readfd, streamfpopen(readfp), initial_offset, ahead, &is_parsing, ¶llel_parser, parser_created, reading.c_str(), &readers, &progress_seq, exclude, include, exclude_all, basezoom, layer, layermaps, initialized.data(), initial_x.data(), initial_y.data(), maxzoom, sources[layer].layer, gamma != 0, attribute_types, read_parallel_this, &dist_sum, &dist_count, &area_sum, guess_maxzoom, prefilter != NULL || postfilter != NULL); if (parser_created) { if (pthread_join(parallel_parser, NULL) != 0) { @@ -1935,27 +1925,26 @@ std::pair read_input(std::vector &sources, char *fname, i // segment+offset to find the data. // 2 * CPUS: One per input thread, one per tiling thread - long long pool_off[2 * CPUS]; + std::vector pool_off(2 * CPUS); for (size_t i = 0; i < 2 * CPUS; i++) { pool_off[i] = 0; } - char poolname[strlen(tmpdir) + strlen("/pool.XXXXXXXX") + 1]; - snprintf(poolname, sizeof(poolname), "%s%s", tmpdir, "/pool.XXXXXXXX"); + std::string poolname = std::string(tmpdir) + "/pool.XXXXXXXX"; - int poolfd = mkstemp_cloexec(poolname); + int poolfd = mkstemp_cloexec(&poolname[0]); if (poolfd < 0) { - perror(poolname); + perror(poolname.c_str()); exit(EXIT_OPEN); } - FILE *poolfile = fopen_oflag(poolname, "wb", O_WRONLY | O_CLOEXEC); + FILE *poolfile = fopen_oflag(poolname.c_str(), "wb", O_WRONLY | O_CLOEXEC); if (poolfile == NULL) { - perror(poolname); + perror(poolname.c_str()); exit(EXIT_OPEN); } - unlink(poolname); + unlink(poolname.c_str()); std::atomic poolpos(0); for (size_t i = 0; i < CPUS; i++) { @@ -2183,36 +2172,34 @@ std::pair read_input(std::vector &sources, char *fname, i fprintf(stderr, "Merging index \r"); } - char indexname[strlen(tmpdir) + strlen("/index.XXXXXXXX") + 1]; - snprintf(indexname, sizeof(indexname), "%s%s", tmpdir, "/index.XXXXXXXX"); + std::string indexname = std::string(tmpdir) + "/index.XXXXXXXX"; - int indexfd = mkstemp_cloexec(indexname); + int indexfd = mkstemp_cloexec(&indexname[0]); if (indexfd < 0) { - perror(indexname); + perror(indexname.c_str()); exit(EXIT_OPEN); } - FILE *indexfile = fopen_oflag(indexname, "wb", O_WRONLY | O_CLOEXEC); + FILE *indexfile = fopen_oflag(indexname.c_str(), "wb", O_WRONLY | O_CLOEXEC); if (indexfile == NULL) { - perror(indexname); + perror(indexname.c_str()); exit(EXIT_OPEN); } - unlink(indexname); + unlink(indexname.c_str()); - char geomname[strlen(tmpdir) + strlen("/geom.XXXXXXXX") + 1]; - snprintf(geomname, sizeof(geomname), "%s%s", tmpdir, "/geom.XXXXXXXX"); + std::string geomname = std::string(tmpdir) + "/geom.XXXXXXXX"; - int geomfd = mkstemp_cloexec(geomname); + int geomfd = mkstemp_cloexec(&geomname[0]); if (geomfd < 0) { - perror(geomname); + perror(geomname.c_str()); exit(EXIT_CLOSE); } - FILE *geomfile = fopen_oflag(geomname, "wb", O_WRONLY | O_CLOEXEC); + FILE *geomfile = fopen_oflag(geomname.c_str(), "wb", O_WRONLY | O_CLOEXEC); if (geomfile == NULL) { - perror(geomname); + perror(geomname.c_str()); exit(EXIT_OPEN); } - unlink(geomname); + unlink(geomname.c_str()); unsigned iz = 0, ix = 0, iy = 0; choose_first_zoom(file_bbox, file_bbox1, file_bbox2, readers, &iz, &ix, &iy, minzoom, buffer); @@ -2699,8 +2686,8 @@ std::pair read_input(std::vector &sources, char *fname, i madvise(geom, indexpos, MADV_SEQUENTIAL); madvise(geom, indexpos, MADV_WILLNEED); - struct drop_state ds[maxzoom + 1]; - prep_drop_states(ds, maxzoom, basezoom, droprate); + std::vector ds(maxzoom + 1); + prep_drop_states(ds.data(), maxzoom, basezoom, droprate); if (drop_denser > 0) { std::vector ddv; @@ -2718,7 +2705,7 @@ std::pair read_input(std::vector &sources, char *fname, i previndex = map[ip].ix; } else { - int feature_minzoom = calc_feature_minzoom(&map[ip], ds, maxzoom, gamma); + int feature_minzoom = calc_feature_minzoom(&map[ip], ds.data(), maxzoom, gamma); geom[map[ip].end - 1] = feature_minzoom; } } @@ -2743,7 +2730,7 @@ std::pair read_input(std::vector &sources, char *fname, i if (ip > 0 && map[ip].start != map[ip - 1].end) { fprintf(stderr, "Mismatched index at %lld: %lld vs %lld\n", ip, map[ip].start, map[ip].end); } - int feature_minzoom = calc_feature_minzoom(&map[ip], ds, maxzoom, gamma); + int feature_minzoom = calc_feature_minzoom(&map[ip], ds.data(), maxzoom, gamma); geom[map[ip].end - 1] = feature_minzoom; } } @@ -2766,8 +2753,8 @@ std::pair read_input(std::vector &sources, char *fname, i exit(EXIT_STAT); } - int fd[TEMP_FILES]; - off_t size[TEMP_FILES]; + std::vector fd(TEMP_FILES); + std::vector size(TEMP_FILES); fd[0] = geomfd; size[0] = geomst.st_size; @@ -2780,7 +2767,7 @@ std::pair read_input(std::vector &sources, char *fname, i std::atomic midx(0); std::atomic midy(0); std::vector strategies; - int written = traverse_zooms(fd, size, stringpool, &midx, &midy, maxzoom, minzoom, outdb, outdir, buffer, fname, tmpdir, gamma, full_detail, low_detail, min_detail, pool_off, initial_x, initial_y, simplification, maxzoom_simplification, layermaps, prefilter, postfilter, attribute_accum, filter, strategies, iz, shared_nodes_map, nodepos, shared_nodes_bloom, basezoom, droprate, unidecode_data, &drop_by_attribute_as_needed_attribute, drop_by_attribute_descending); + int written = traverse_zooms(fd.data(), size.data(), stringpool, &midx, &midy, maxzoom, minzoom, outdb, outdir, buffer, fname, tmpdir, gamma, full_detail, low_detail, min_detail, pool_off.data(), initial_x.data(), initial_y.data(), simplification, maxzoom_simplification, layermaps, prefilter, postfilter, attribute_accum, filter, strategies, iz, shared_nodes_map, nodepos, shared_nodes_bloom, basezoom, droprate, unidecode_data, &drop_by_attribute_as_needed_attribute, drop_by_attribute_descending); if (maxzoom != written) { if (written > minzoom) { diff --git a/mbtiles.cpp b/mbtiles.cpp index c7a615b0..4a3ea9a9 100644 --- a/mbtiles.cpp +++ b/mbtiles.cpp @@ -701,7 +701,10 @@ metadata make_metadata(const char *fname, int minzoom, int maxzoom, double minla m.strategies_json = stringify_strategies(strategies); if (std::isinf(droprate)) { - droprate = LLONG_MAX; + // JSON has no representation for infinity, so substitute a huge + // finite value. The cast is explicit because LLONG_MAX itself is + // not representable as a double and rounds up to 2^63. + droprate = (double) LLONG_MAX; } if (basezoom != maxzoom || droprate != 2.5 || retain_points_multiplier != 1) { m.decisions_json = std::string("{") + diff --git a/mvt.hpp b/mvt.hpp index 8f24fc68..0c36b74f 100644 --- a/mvt.hpp +++ b/mvt.hpp @@ -93,12 +93,21 @@ struct mvt_value { long long sint_value; bool bool_value; int null_value; + // Initializing string_value initializes the union's full width, which + // the static_assert below checks. Setting only a narrower member (a + // double, say) would leave the remaining bytes indeterminate, and the + // implicit copy constructor copies the union as a whole, so those + // bytes get read even when they aren't the member in use. struct { size_t off; size_t len; - } string_value; + } string_value = {0, 0}; } numeric_value; + static_assert(sizeof(numeric_value) == sizeof(numeric_value.string_value), + "string_value must span the whole union, since its default member " + "initializer is what initializes the union"); + std::string get_string_value() const { if (type == mvt_string) { return std::string(*s, numeric_value.string_value.off, numeric_value.string_value.len); diff --git a/serial.cpp b/serial.cpp index 5afc2ed2..73868194 100644 --- a/serial.cpp +++ b/serial.cpp @@ -665,10 +665,18 @@ int serialize_feature(struct serialization_state *sst, serial_feature &sf, std:: // VT_POINT extent will be calculated in write_tile from the distance between adjacent features. } - if (extent <= LLONG_MAX) { + // Clamp before converting, since converting a double that is out of range + // for a long long is undefined. The bounds are asymmetric: LLONG_MAX is not + // representable as a double and rounds up to 2^63, so the upper bound has to + // be exclusive, while LLONG_MIN is exactly -2^63 and so can be included. + // Areas are signed, so holes that outweigh their rings can make this + // negative. + if (extent >= (double) LLONG_MIN && extent < (double) LLONG_MAX) { sf.extent = (long long) extent; + } else if (extent < 0) { + sf.extent = LLONG_MIN; } else { - sf.extent = LLONG_MAX; + sf.extent = LLONG_MAX; // also the NaN case } if (sst->want_dist && sf.t == VT_POLYGON) { diff --git a/tile-join.cpp b/tile-join.cpp index 921a38b9..017e04b8 100644 --- a/tile-join.cpp +++ b/tile-join.cpp @@ -893,7 +893,7 @@ void *join_worker(void *v) { } void dispatch_tasks(std::map> &tasks, std::vector> &layermaps, sqlite3 *outdb, const char *outdir, std::vector &header, std::map> &mapping, sqlite3 *db, std::set &exclude, std::set &include, int ifmatched, std::set &keep_layers, std::set &remove_layers, json_object *filter, struct tileset_reader *readers, double *minlat, double *minlon, double *maxlat, double *maxlon, double *minlon2, double *maxlon2) { - pthread_t pthreads[CPUS]; + std::vector pthreads(CPUS); std::vector args; for (size_t i = 0; i < CPUS; i++) { diff --git a/tile.cpp b/tile.cpp index ede49e89..a80be63a 100644 --- a/tile.cpp +++ b/tile.cpp @@ -61,9 +61,6 @@ extern "C" { #define COORD_OFFSET (4LL << 32) #define SHIFT_RIGHT(a) ((long long) std::round((double) (a) / (1LL << geometry_scale))) -#define XSTRINGIFY(s) STRINGIFY(s) -#define STRINGIFY(s) #s - pthread_mutex_t db_lock = PTHREAD_MUTEX_INITIALIZER; pthread_mutex_t var_lock = PTHREAD_MUTEX_INITIALIZER; pthread_mutex_t task_lock = PTHREAD_MUTEX_INITIALIZER; @@ -1747,8 +1744,8 @@ long long write_tile(decompressor *geoms, std::atomic *geompos_in, ch key_pool key_pool; - std::atomic within[child_shards]; - long long start_geompos[child_shards]; + std::vector > within(child_shards); + std::vector start_geompos(child_shards); for (size_t i = 0; i < (size_t) child_shards; i++) { within[i] = false; start_geompos[i] = -1; @@ -1813,10 +1810,10 @@ long long write_tile(decompressor *geoms, std::atomic *geompos_in, ch rpa.along = along; rpa.alongminus = alongminus; rpa.buffer = buffer; - rpa.within = within; + rpa.within = within.data(); rpa.geomfile = geomfile; rpa.geompos = geompos; - rpa.start_geompos = start_geompos; + rpa.start_geompos = start_geompos.data(); rpa.oprogress = &oprogress; rpa.todo = todo; rpa.fname = fname; @@ -1861,7 +1858,7 @@ long long write_tile(decompressor *geoms, std::atomic *geompos_in, ch ssize_t which_serial_feature = -1; if (prefilter == NULL) { - sf = next_feature(geoms, geompos_in, z, tx, ty, initial_x, initial_y, &original_features, &unclipped_features, nextzoom, maxzoom, minzoom, max_zoom_increment, pass, along, alongminus, buffer, within, geomfile, geompos, start_geompos, &oprogress, todo, fname, child_shards, filter, global_stringpool, pool_off, layer_unmaps, first_time, compressed_input, &multiplier_state, tile_stringpool, unidecode_data, next_feature_state, arg->droprate); + sf = next_feature(geoms, geompos_in, z, tx, ty, initial_x, initial_y, &original_features, &unclipped_features, nextzoom, maxzoom, minzoom, max_zoom_increment, pass, along, alongminus, buffer, within.data(), geomfile, geompos, start_geompos.data(), &oprogress, todo, fname, child_shards, filter, global_stringpool, pool_off, layer_unmaps, first_time, compressed_input, &multiplier_state, tile_stringpool, unidecode_data, next_feature_state, arg->droprate); } else { sf = parse_feature(prefilter_jp, z, tx, ty, layermaps, tiling_seg, layer_unmaps, postfilter != NULL, key_pool); } @@ -2398,7 +2395,7 @@ long long write_tile(decompressor *geoms, std::atomic *geompos_in, ch if (p.clustered > 0) { serial_val sv, sv2, sv3, sv4; long long point_count = p.clustered + 1; - char abbrev[20]; // to_string(LLONG_MAX).length() / 1000 + 1; + char abbrev[24]; // fits "%lld" of any long long, including the sign and the NUL p.full_keys.push_back(key_pool.pool("clustered")); sv.type = mvt_bool; @@ -2448,7 +2445,7 @@ long long write_tile(decompressor *geoms, std::atomic *geompos_in, ch } { - pthread_t pthreads[tasks]; + std::vector pthreads(tasks); std::vector args; args.resize(tasks); for (int i = 0; i < tasks; i++) { @@ -3252,28 +3249,27 @@ int traverse_zooms(int *geomfd, off_t *geom_size, char *global_stringpool, std:: for (z = iz; z <= maxzoom; z++) { std::atomic most(0); - compressor compressors[TEMP_FILES]; - compressor *sub[TEMP_FILES]; - std::atomic subpos[TEMP_FILES]; - int subfd[TEMP_FILES]; + std::vector compressors(TEMP_FILES); + std::vector sub(TEMP_FILES); + std::vector > subpos(TEMP_FILES); + std::vector subfd(TEMP_FILES); for (size_t j = 0; j < TEMP_FILES; j++) { - char geomname[strlen(tmpdir) + strlen("/geom.XXXXXXXX" XSTRINGIFY(INT_MAX)) + 1]; - snprintf(geomname, sizeof(geomname), "%s/geom%zu.XXXXXXXX", tmpdir, j); - subfd[j] = mkstemp_cloexec(geomname); - // printf("%s\n", geomname); + std::string geomname = std::string(tmpdir) + "/geom" + std::to_string(j) + ".XXXXXXXX"; + subfd[j] = mkstemp_cloexec(&geomname[0]); + // printf("%s\n", geomname.c_str()); if (subfd[j] < 0) { - perror(geomname); + perror(geomname.c_str()); exit(EXIT_OPEN); } - FILE *fp = fopen_oflag(geomname, "wb", O_WRONLY | O_CLOEXEC); + FILE *fp = fopen_oflag(geomname.c_str(), "wb", O_WRONLY | O_CLOEXEC); if (fp == NULL) { - perror(geomname); + perror(geomname.c_str()); exit(EXIT_OPEN); } compressors[j] = compressor(fp); sub[j] = &compressors[j]; subpos[j] = 0; - unlink(geomname); + unlink(geomname.c_str()); } size_t useful_threads = 0; @@ -3341,7 +3337,7 @@ int traverse_zooms(int *geomfd, off_t *geom_size, char *global_stringpool, std:: std::set skip_children_out; for (size_t pass = 0;; pass++) { - pthread_t pthreads[threads]; + std::vector pthreads(threads); std::vector args; args.resize(threads); std::atomic running(threads); @@ -3360,8 +3356,8 @@ int traverse_zooms(int *geomfd, off_t *geom_size, char *global_stringpool, std:: args[thread].outdir = outdir; args[thread].buffer = buffer; args[thread].fname = fname; - args[thread].geomfile = sub + thread * (TEMP_FILES / threads); - args[thread].geompos = subpos + thread * (TEMP_FILES / threads); + args[thread].geomfile = sub.data() + thread * (TEMP_FILES / threads); + args[thread].geompos = subpos.data() + thread * (TEMP_FILES / threads); args[thread].todo = todo; args[thread].along = &along; // locked with var_lock args[thread].gamma = zoom_gamma; diff --git a/version.hpp b/version.hpp index 8974b68f..71b353ad 100644 --- a/version.hpp +++ b/version.hpp @@ -1,6 +1,6 @@ #ifndef VERSION_HPP #define VERSION_HPP -#define VERSION "v2.80.0" +#define VERSION "v2.81.0" #endif