From c594668c2bd79a16f2c299b728346e50e2ac0899 Mon Sep 17 00:00:00 2001 From: Matt Bhagat-Conway Date: Wed, 29 Oct 2025 16:50:58 -0400 Subject: [PATCH 1/6] add boundschecks to fix #35 --- src/LibSpatialIndex.jl | 14 +++++++++++--- test/runtests.jl | 17 +++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/LibSpatialIndex.jl b/src/LibSpatialIndex.jl index 39917ad..19d99bd 100644 --- a/src/LibSpatialIndex.jl +++ b/src/LibSpatialIndex.jl @@ -179,8 +179,10 @@ module LibSpatialIndex minvalues::Vector{Float64}, maxvalues::Vector{Float64} ) + length(minvalues) == rtree.ndim || throw(DimensionMismatch("Minimum values must have same length as RTree dimensions")) + length(maxvalues) == rtree.ndim || throw(DimensionMismatch("Maximum values must have same length as RTree dimensions")) C.Index_InsertData(rtree.index, Int64(id), pointer(minvalues), - pointer(maxvalues), UInt32(length(minvalues)), Ptr{UInt8}(0), Cint(0) + pointer(maxvalues), UInt32(rtree.ndim), Ptr{UInt8}(0), Cint(0) ) end function insert!(rtree::RTree, id::Integer, extent::GI.Extent) @@ -213,10 +215,13 @@ module LibSpatialIndex minvalues::Vector{Float64}, maxvalues::Vector{Float64} ) + length(minvalues) == rtree.ndim || throw(DimensionMismatch("Minimum values must have same length as RTree dimensions")) + length(maxvalues) == rtree.ndim || throw(DimensionMismatch("Maximum values must have same length as RTree dimensions")) + items = Ref{Ptr{Int64}}() nresults = Ref{UInt64}() result = C.Index_Intersects_id(rtree.index, pointer(minvalues), - pointer(maxvalues), UInt32(length(minvalues)), items, nresults + pointer(maxvalues), UInt32(rtree.ndim), items, nresults ) _checkresult(result, "Index_Intersects_id: Failed to evaluate") unsafe_wrap(Array, items[], nresults[]) @@ -260,10 +265,13 @@ module LibSpatialIndex maxvalues::Vector{Float64}, k::Integer ) + length(minvalues) == rtree.ndim || throw(DimensionMismatch("Minimum values must have same length as RTree dimensions")) + length(maxvalues) == rtree.ndim || throw(DimensionMismatch("Maximum values must have same length as RTree dimensions")) + items = Ref{Ptr{Int64}}() nresults = Ref{UInt64}(k) result = C.Index_NearestNeighbors_id(rtree.index, pointer(minvalues), - pointer(maxvalues), UInt32(length(minvalues)), items, nresults) + pointer(maxvalues), UInt32(rtree.ndim), items, nresults) _checkresult(result, "Index_NearestNeighbors_id: Failed to evaluate") unsafe_wrap(Array, items[], nresults[]) end diff --git a/test/runtests.jl b/test/runtests.jl index b5f1b55..0ca41ca 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -70,6 +70,23 @@ end @test sort(SI.knn(rtree, (2.0, 2.0), 1)) == [2] end +@testset "Bounds checks" begin + # confirm that bounds checks on the Julia side are working + tree = SI.RTree(2) + @test_throws DimensionMismatch SI.insert!(tree, 0, [0.0], [0.0, 0.1]) + @test_throws DimensionMismatch SI.insert!(tree, 0, [0.0, 1.0], [0.1]) + # should throw even if they're the same length if they don't match dimensions + @test_throws DimensionMismatch SI.insert!(tree, 0, [0.0, 0.1, 1.1], [0.0, 0.1, 1.1]) + + @test_throws DimensionMismatch SI.intersects(tree, [0.0], [0.0, 0.1]) + @test_throws DimensionMismatch SI.intersects(tree, [0.0, 1.0], [0.1]) + @test_throws DimensionMismatch SI.intersects(tree, [0.0, 0.1, 1.1], [0.0, 0.1, 1.1]) + + @test_throws DimensionMismatch SI.knn(tree, [0.0], [0.0, 0.1], 1) + @test_throws DimensionMismatch SI.knn(tree, [0.0, 1.0], [0.1], 1) + @test_throws DimensionMismatch SI.knn(tree, [0.0, 0.1, 1.1], [0.0, 0.1, 1.1], 1) +end + @testset "Aqua" begin Aqua.test_all(LibSpatialIndex) end From 1eedd20730a53f014c36fcb17434a6cf6e140ca8 Mon Sep 17 00:00:00 2001 From: Matt Bhagat-Conway Date: Thu, 30 Oct 2025 09:05:11 -0400 Subject: [PATCH 2/6] correct pointer handling for GetBounds in test --- test/runtests.jl | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index 0ca41ca..de16f04 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -28,11 +28,21 @@ import Aqua @test nresults[] == 1 # bounds() - pmins = zeros(2) - pmaxs = zeros(2) + pmins_p = Ref{Ptr{Float64}}() + pmaxs_p = Ref{Ptr{Float64}}() ndims = Ref{UInt32}() - SI.C.Index_GetBounds(idx, pointer_from_objref(pmins), pointer_from_objref(pmaxs), ndims); + + SI.C.Index_GetBounds(idx, pmins_p, pmaxs_p, ndims); + + # somehow in here the object gets messed up, but only during testing - wtf? + # but only on 1.11 and 1.12? gotta be some kind of bug in Julia itself + # Does not happen when running from the REPL or the command line, even in test mode + # Only happens with Pkg.test() @test ndims[] == 2 + + pmins = unsafe_wrap(Vector{Float64}, pmins_p[], ndims[], own=true) + pmaxs = unsafe_wrap(Vector{Float64}, pmaxs_p[], ndims[], own=true) + @test isapprox(pmins, [0.5, 0.5]) @test isapprox(pmaxs, [0.5, 0.5]) end From 45d7920cb4ce2f26654a378b336aa0ed72b70601 Mon Sep 17 00:00:00 2001 From: Matt Bhagat-Conway Date: Thu, 30 Oct 2025 09:05:38 -0400 Subject: [PATCH 3/6] bump aqua version and add Test compat to solve aqua error --- Project.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Project.toml b/Project.toml index 12d687a..02569bb 100644 --- a/Project.toml +++ b/Project.toml @@ -8,10 +8,11 @@ GeoInterface = "cf35fbd7-0cd7-5166-be24-54bfbe79505f" LibSpatialIndex_jll = "00e98e2a-4326-5239-88cb-15dcbe1c18d0" [compat] -Aqua = "0.7" +Aqua = "0.8" GeoInterface = "1" LibSpatialIndex_jll = "1.9" julia = "1.6" +Test = "1.6" [extras] Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595" From ed8fb0a99d78d18b2fc06b29289dbfef6fb1bd2a Mon Sep 17 00:00:00 2001 From: Matt Bhagat-Conway Date: Thu, 30 Oct 2025 09:15:54 -0400 Subject: [PATCH 4/6] place all tests inside outer testset so tests don't stop when error encountered (JuliaLang/julia#21594) --- test/runtests.jl | 192 ++++++++++++++++++++++++----------------------- 1 file changed, 97 insertions(+), 95 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index de16f04..06f6100 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -4,99 +4,101 @@ import GeoInterface as GI import LibSpatialIndex as SI import Aqua -@testset "Simple Tutorial" begin - # based on https://github.com/libspatialindex/libspatialindex/wiki/Simple-Tutorial - println("Testing LibSpatialIndex v$(SI.version())") - props = SI.C.IndexProperty_Create() - SI.C.IndexProperty_SetIndexType(props, SI.C.RT_RTree) - SI.C.IndexProperty_SetIndexStorage(props, SI.C.RT_Memory) - idx = SI.C.Index_Create(props) - SI.C.IndexProperty_Destroy(props) - @test Bool(SI.C.Index_IsValid(idx)) - - # load() - min = [0.5, 0.5] - max = [0.5, 0.5] - SI.C.Index_InsertData(idx, 1, min, max, 2, Ptr{UInt8}(C_NULL), 0) - - # query() - min = [0.0, 0.0] - max = [1.0, 1.0] - ndims = UInt32(2) - nresults = Ref{UInt64}() - SI.C.Index_Intersects_count(idx, min, max, ndims, nresults) - @test nresults[] == 1 - - # bounds() - pmins_p = Ref{Ptr{Float64}}() - pmaxs_p = Ref{Ptr{Float64}}() - ndims = Ref{UInt32}() - - SI.C.Index_GetBounds(idx, pmins_p, pmaxs_p, ndims); - - # somehow in here the object gets messed up, but only during testing - wtf? - # but only on 1.11 and 1.12? gotta be some kind of bug in Julia itself - # Does not happen when running from the REPL or the command line, even in test mode - # Only happens with Pkg.test() - @test ndims[] == 2 - - pmins = unsafe_wrap(Vector{Float64}, pmins_p[], ndims[], own=true) - pmaxs = unsafe_wrap(Vector{Float64}, pmaxs_p[], ndims[], own=true) - - @test isapprox(pmins, [0.5, 0.5]) - @test isapprox(pmaxs, [0.5, 0.5]) -end - -@testset "Simple Operations" begin - rtree = SI.RTree(2) - result = SI.insert!(rtree, 1, [0.,0.], [1.,1.]) - @test result == SI.C.RT_None - result = SI.insert!(rtree, 2, [0.,0.], [2.,2.]) - @test result == SI.C.RT_None - @test SI.intersects(rtree, [0.,0.],[1.,1.]) == [1,2] - @test SI.intersects(rtree, [0.,0.]) == [1,2] - @test SI.intersects(rtree, [2.,2.]) == [2] - @test SI.intersects(rtree, [1.5,1.5],[2.,2.]) == [2] - @test sort(SI.knn(rtree, [2.,2.],[2.,2.], 1)) == [2] - @test sort(SI.knn(rtree, [2.,2.],[2.,2.], 2)) == [1,2] - @test sort(SI.knn(rtree, [2.,2.],[2.,2.], 3)) == [1,2] - @test sort(SI.knn(rtree, [2.,2.], 1)) == [2] - @test sort(SI.knn(rtree, [2.,2.], 2)) == [1,2] -end - -@testset "GeoInterface/Extents Operations" begin - rtree = SI.RTree(2) - result = SI.insert!(rtree, 1, GI.Extent(X=(0.0, 1.0), Y=(0.0, 1.0))) - @test result == SI.C.RT_None - - polygon = GI.Polygon([GI.LinearRing([(0.0, 0.0), (0.5, 0.0), (2.0, 0.5), (0.0, 2.0), (0.0, 0.0)])]) - result = SI.insert!(rtree, 2, polygon) - - @test result == SI.C.RT_None - @test SI.intersects(rtree, GI.LineString([(0.0, 0.0), (1.0, 1.0)])) == [1, 2] - @test SI.intersects(rtree, GI.Point(0.0, 0.0)) == [1, 2] - @test SI.intersects(rtree, (X=2.0, Y=2.0)) == [2] - @test sort(SI.knn(rtree, GI.Extent(X=(2.0, 2.0), Y=(2.0, 2.0)), 1)) == [2] - @test sort(SI.knn(rtree, (2.0, 2.0), 1)) == [2] -end - -@testset "Bounds checks" begin - # confirm that bounds checks on the Julia side are working - tree = SI.RTree(2) - @test_throws DimensionMismatch SI.insert!(tree, 0, [0.0], [0.0, 0.1]) - @test_throws DimensionMismatch SI.insert!(tree, 0, [0.0, 1.0], [0.1]) - # should throw even if they're the same length if they don't match dimensions - @test_throws DimensionMismatch SI.insert!(tree, 0, [0.0, 0.1, 1.1], [0.0, 0.1, 1.1]) - - @test_throws DimensionMismatch SI.intersects(tree, [0.0], [0.0, 0.1]) - @test_throws DimensionMismatch SI.intersects(tree, [0.0, 1.0], [0.1]) - @test_throws DimensionMismatch SI.intersects(tree, [0.0, 0.1, 1.1], [0.0, 0.1, 1.1]) - - @test_throws DimensionMismatch SI.knn(tree, [0.0], [0.0, 0.1], 1) - @test_throws DimensionMismatch SI.knn(tree, [0.0, 1.0], [0.1], 1) - @test_throws DimensionMismatch SI.knn(tree, [0.0, 0.1, 1.1], [0.0, 0.1, 1.1], 1) -end - -@testset "Aqua" begin - Aqua.test_all(LibSpatialIndex) +@testset "LibSpatialIndex" begin + @testset "Simple Tutorial" begin + # based on https://github.com/libspatialindex/libspatialindex/wiki/Simple-Tutorial + println("Testing LibSpatialIndex v$(SI.version())") + props = SI.C.IndexProperty_Create() + SI.C.IndexProperty_SetIndexType(props, SI.C.RT_RTree) + SI.C.IndexProperty_SetIndexStorage(props, SI.C.RT_Memory) + idx = SI.C.Index_Create(props) + SI.C.IndexProperty_Destroy(props) + @test Bool(SI.C.Index_IsValid(idx)) + + # load() + min = [0.5, 0.5] + max = [0.5, 0.5] + SI.C.Index_InsertData(idx, 1, min, max, 2, Ptr{UInt8}(C_NULL), 0) + + # query() + min = [0.0, 0.0] + max = [1.0, 1.0] + ndims = UInt32(2) + nresults = Ref{UInt64}() + SI.C.Index_Intersects_count(idx, min, max, ndims, nresults) + @test nresults[] == 1 + + # bounds() + pmins_p = Ref{Ptr{Float64}}() + pmaxs_p = Ref{Ptr{Float64}}() + ndims = Ref{UInt32}() + + SI.C.Index_GetBounds(idx, pmins_p, pmaxs_p, ndims); + + # somehow in here the object gets messed up, but only during testing - wtf? + # but only on 1.11 and 1.12? gotta be some kind of bug in Julia itself + # Does not happen when running from the REPL or the command line, even in test mode + # Only happens with Pkg.test() + @test ndims[] == 2 + + pmins = unsafe_wrap(Vector{Float64}, pmins_p[], ndims[], own=true) + pmaxs = unsafe_wrap(Vector{Float64}, pmaxs_p[], ndims[], own=true) + + @test isapprox(pmins, [0.5, 0.5]) + @test isapprox(pmaxs, [0.5, 0.5]) + end + + @testset "Simple Operations" begin + rtree = SI.RTree(2) + result = SI.insert!(rtree, 1, [0.,0.], [1.,1.]) + @test result == SI.C.RT_None + result = SI.insert!(rtree, 2, [0.,0.], [2.,2.]) + @test result == SI.C.RT_None + @test SI.intersects(rtree, [0.,0.],[1.,1.]) == [1,2] + @test SI.intersects(rtree, [0.,0.]) == [1,2] + @test SI.intersects(rtree, [2.,2.]) == [2] + @test SI.intersects(rtree, [1.5,1.5],[2.,2.]) == [2] + @test sort(SI.knn(rtree, [2.,2.],[2.,2.], 1)) == [2] + @test sort(SI.knn(rtree, [2.,2.],[2.,2.], 2)) == [1,2] + @test sort(SI.knn(rtree, [2.,2.],[2.,2.], 3)) == [1,2] + @test sort(SI.knn(rtree, [2.,2.], 1)) == [2] + @test sort(SI.knn(rtree, [2.,2.], 2)) == [1,2] + end + + @testset "GeoInterface/Extents Operations" begin + rtree = SI.RTree(2) + result = SI.insert!(rtree, 1, GI.Extent(X=(0.0, 1.0), Y=(0.0, 1.0))) + @test result == SI.C.RT_None + + polygon = GI.Polygon([GI.LinearRing([(0.0, 0.0), (0.5, 0.0), (2.0, 0.5), (0.0, 2.0), (0.0, 0.0)])]) + result = SI.insert!(rtree, 2, polygon) + + @test result == SI.C.RT_None + @test SI.intersects(rtree, GI.LineString([(0.0, 0.0), (1.0, 1.0)])) == [1, 2] + @test SI.intersects(rtree, GI.Point(0.0, 0.0)) == [1, 2] + @test SI.intersects(rtree, (X=2.0, Y=2.0)) == [2] + @test sort(SI.knn(rtree, GI.Extent(X=(2.0, 2.0), Y=(2.0, 2.0)), 1)) == [2] + @test sort(SI.knn(rtree, (2.0, 2.0), 1)) == [2] + end + + @testset "Bounds checks" begin + # confirm that bounds checks on the Julia side are working + tree = SI.RTree(2) + @test_throws DimensionMismatch SI.insert!(tree, 0, [0.0], [0.0, 0.1]) + @test_throws DimensionMismatch SI.insert!(tree, 0, [0.0, 1.0], [0.1]) + # should throw even if they're the same length if they don't match dimensions + @test_throws DimensionMismatch SI.insert!(tree, 0, [0.0, 0.1, 1.1], [0.0, 0.1, 1.1]) + + @test_throws DimensionMismatch SI.intersects(tree, [0.0], [0.0, 0.1]) + @test_throws DimensionMismatch SI.intersects(tree, [0.0, 1.0], [0.1]) + @test_throws DimensionMismatch SI.intersects(tree, [0.0, 0.1, 1.1], [0.0, 0.1, 1.1]) + + @test_throws DimensionMismatch SI.knn(tree, [0.0], [0.0, 0.1], 1) + @test_throws DimensionMismatch SI.knn(tree, [0.0, 1.0], [0.1], 1) + @test_throws DimensionMismatch SI.knn(tree, [0.0, 0.1, 1.1], [0.0, 0.1, 1.1], 1) + end + + @testset "Aqua" begin + Aqua.test_all(LibSpatialIndex) + end end From 7c966390500598183acc385a75d0dece293d9797 Mon Sep 17 00:00:00 2001 From: Matt Bhagat-Conway Date: Thu, 30 Oct 2025 10:15:23 -0400 Subject: [PATCH 5/6] rm spurious comment --- test/runtests.jl | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index 06f6100..6af8bea 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -35,12 +35,7 @@ import Aqua SI.C.Index_GetBounds(idx, pmins_p, pmaxs_p, ndims); - # somehow in here the object gets messed up, but only during testing - wtf? - # but only on 1.11 and 1.12? gotta be some kind of bug in Julia itself - # Does not happen when running from the REPL or the command line, even in test mode - # Only happens with Pkg.test() @test ndims[] == 2 - pmins = unsafe_wrap(Vector{Float64}, pmins_p[], ndims[], own=true) pmaxs = unsafe_wrap(Vector{Float64}, pmaxs_p[], ndims[], own=true) From 962020cea4dac3d044c646e22b52586f7415ed18 Mon Sep 17 00:00:00 2001 From: Matt Bhagat-Conway Date: Thu, 30 Oct 2025 10:26:19 -0400 Subject: [PATCH 6/6] pass an actual null pointer to insert --- src/LibSpatialIndex.jl | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/LibSpatialIndex.jl b/src/LibSpatialIndex.jl index 19d99bd..e520699 100644 --- a/src/LibSpatialIndex.jl +++ b/src/LibSpatialIndex.jl @@ -181,8 +181,11 @@ module LibSpatialIndex ) length(minvalues) == rtree.ndim || throw(DimensionMismatch("Minimum values must have same length as RTree dimensions")) length(maxvalues) == rtree.ndim || throw(DimensionMismatch("Maximum values must have same length as RTree dimensions")) - C.Index_InsertData(rtree.index, Int64(id), pointer(minvalues), - pointer(maxvalues), UInt32(rtree.ndim), Ptr{UInt8}(0), Cint(0) + + # pass C_NULL as data - it should not be dereferenced, but making it null hopefully means if it is + # it will be easier to debug + C.Index_InsertData(rtree.index, Int64(id), minvalues, + maxvalues, UInt32(rtree.ndim), C_NULL, Cint(0) ) end function insert!(rtree::RTree, id::Integer, extent::GI.Extent) @@ -220,8 +223,8 @@ module LibSpatialIndex items = Ref{Ptr{Int64}}() nresults = Ref{UInt64}() - result = C.Index_Intersects_id(rtree.index, pointer(minvalues), - pointer(maxvalues), UInt32(rtree.ndim), items, nresults + result = C.Index_Intersects_id(rtree.index, minvalues, + maxvalues, UInt32(rtree.ndim), items, nresults ) _checkresult(result, "Index_Intersects_id: Failed to evaluate") unsafe_wrap(Array, items[], nresults[]) @@ -270,8 +273,8 @@ module LibSpatialIndex items = Ref{Ptr{Int64}}() nresults = Ref{UInt64}(k) - result = C.Index_NearestNeighbors_id(rtree.index, pointer(minvalues), - pointer(maxvalues), UInt32(rtree.ndim), items, nresults) + result = C.Index_NearestNeighbors_id(rtree.index, minvalues, + maxvalues, UInt32(rtree.ndim), items, nresults) _checkresult(result, "Index_NearestNeighbors_id: Failed to evaluate") unsafe_wrap(Array, items[], nresults[]) end @@ -308,5 +311,4 @@ module LibSpatialIndex end _not_point_or_ext_error() = throw(ArgumentError("object is not a point, and does not have an extent")) - -end # module +end # module \ No newline at end of file