Skip to content

lib/vector/dgl: use fixed width integer types for dglInt[32|64]_t - #7962

Open
nilason wants to merge 2 commits into
OSGeo:mainfrom
nilason:fix_libvector-dgl_types
Open

nilason wants to merge 2 commits into
OSGeo:mainfrom
nilason:fix_libvector-dgl_types

Conversation

@nilason

@nilason nilason commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

The types dglInt32_t and dglInt64_t were originally intended to be fixed-width integers, as indicated by the original commit from 2002 [1]. At the time, C99 (and <stdint.h>) was new and not yet widely supported, hence the use of long and long long. Today, long is 64 bits on platforms using the LP64 data model, including 64-bit Linux and macOS, while it remains 32 bits on Windows.

This update restores the intended 32-bit width of dglInt32_t and uses an explicit 64-bit integer type for dglInt64_t.

[1] OSGeo/grass-legacy@0f9fd9f#diff-311056b41101fbbecd4477b59c9dee3ddf50f64f38926d242790c78489e306ac

The types dglInt32_t and dglInt64_t were originally intended to be
fixed-width integers, as indicated by the original commit from 2002 [1].
At the time, C99 (and <stdint.h>) was new and not yet widely supported,
hence the use of long and long long. Today, long is 64 bits on
platforms using the LP64 data model, including 64-bit Linux and macOS,
while it remains 32 bits on Windows.

This update restores the intended 32-bit width of dglInt32_t and uses
an explicit 64-bit integer type for dglInt64_t.

[1] OSGeo/grass-legacy@0f9fd9f#diff-311056b41101fbbecd4477b59c9dee3ddf50f64f38926d242790c78489e306ac
@github-actions github-actions Bot added vector Related to vector data processing libraries labels Sep 28, 2026
@github-actions github-actions Bot added the C Related code is in C label Sep 28, 2026

@wenzeslaus wenzeslaus left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may need a deeper analysis since long is 64bit for quite some time now on Linux. So there was no size saving until now on Linux (sizeof(dglInt32_t) = 8 on my machine), larger sizes and limits were allowed, but all of that was inconsistent with Windows. This brings the Windows limits to Linux and macOS. Do we want that?

AI-generated review On `main`, `dglInt32_t` is `long`, which is 32-bit on Windows (LLP64) and 64-bit on Linux and macOS (LP64). With `int32_t`, the Windows limits apply everywhere:
  1. Shortest-path distances (nDistance, the accumulators in sp-template.c) are 32-bit. With cost_multip = 1000 (projected location, no cost column), totals above about 2147 km overflow at sp-template.c:302 (fromDist + nEdgeCost) and sp-template.c:194. Testing dglib standalone on x86-64 Linux (gcc 13, -O0 and -O2) with this PR: a 2200 km route was reported as -2095 km. Given a choice between a 4500 km route (3 edges of 1500 km) and a 2100 km route (2 edges of 1050 km), it returned the 4500 km route with a distance of 205 km. The base commit returned 2200 km and the 2100 km route. UBSan reports signed integer overflow at both locations.
  2. dglcosts = (dglInt32_t)costs * 1000 in Vect_graph_add_edge (Vlib/graph.c:131, same in Vect_graph_set_node_costs) overflows for costs above about 2.1 million. Independent of this PR, the cast also drops the fractional part before scaling: 1.7 becomes 1000, not 1700.
  3. iNodeBuffer/iEdgeBuffer and the offset casts in graph_v1.h cap flattened graphs at 2 GiB per buffer.
  4. The shortest-path counts cnt in NetA_betweenness_closeness (neta/centrality.c:139) can overflow on grid-like networks with uniform costs.

An option for a follow-up is to use dglInt64_t for costs and distances while keeping 32-bit for IDs and offsets, or to stop with an error in net_build.c/graph.c when a scaled cost exceeds INT32_MAX.

  1. dglWrite/dglRead (graph_v1.c, graph_v2.c) write sizeof(dglInt32_t) per field, and the header records endianness but not field width. On main, files are therefore not portable between Windows and Linux/macOS, and dgl_swapInt32Bytes (helpers.c) swaps only 4 of the 8 bytes on LP64, so reading a file with the opposite endianness is broken there. The PR fixes both. The trade-off is that files written by older Linux and macOS builds can no longer be read. No tool in this repo calls dglWrite, so a note in the PR description may be enough.

The dglib examples still use %ld with dglInt32_t variables: cr_from_a.c:103, parse.c:282-312, shortest_path.c, view.c. With sscanf, this writes 8 bytes into 4-byte variables on 64-bit Linux. view.c:188 uses %lld for dglInt64_t, which does not match int64_t on Linux (long). Using PRId32 and PRId64 from <inttypes.h> would make all these call sites portable. Using the same macros in the new G_debug calls in Vlib/graph.c and Vlib/net_analyze.c would keep the 32-bit and 64-bit formats consistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C Related code is in C libraries vector Related to vector data processing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants