From fe2131967e8f7bae0dd22fb0710eda620df6d01b Mon Sep 17 00:00:00 2001
From: Hendrik Ranocha <mail@ranocha.de>
Date: Thu, 25 Jul 2024 22:24:18 +0200
Subject: [PATCH 1/8] implement solve2 without workspace arguments

---
 src/solvers/cholmod.jl | 60 +++++++++++++++++++++++++++++++-----------
 1 file changed, 44 insertions(+), 16 deletions(-)

diff --git a/src/solvers/cholmod.jl b/src/solvers/cholmod.jl
index 117d801b..7c640b62 100644
--- a/src/solvers/cholmod.jl
+++ b/src/solvers/cholmod.jl
@@ -747,6 +747,34 @@ for TI ∈ IndexTypes
         Dense{Tv}($(cholname(:solve, TI))(sys, F, B, getcommon($TI)))
     end
 
+    # solve the system and store the result in X
+    function solve2(sys::Integer, F::Factor{Tv, $TI}, B::Dense{Tv}, X::Dense{Tv}) where Tv<:VTypes
+        if size(F, 1) != size(B, 1)
+            throw(DimensionMismatch("Factorization and RHS should have the same number of rows. " *
+                "Factorization has $(size(F, 2)) rows, but RHS has $(size(B, 1)) rows."))
+        end
+        if size(F, 2) != size(X, 1)
+            throw(DimensionMismatch("Factorization and solution should match sizes. " *
+                "Factorization has $(size(F, 1)) columns, but solution has $(size(B, 1)) rows."))
+        end
+        if !issuccess(F)
+            s = unsafe_load(pointer(F))
+            if s.is_ll == 1
+                throw(LinearAlgebra.PosDefException(s.minor))
+            else
+                throw(LinearAlgebra.ZeroPivotException(s.minor))
+            end
+        end
+        X_Handle = pointer(X)
+        Y_Handle = Ptr{LibSuiteSparse.cholmod_dense_struct}()
+        E_Handle = Ptr{LibSuiteSparse.cholmod_dense_struct}()
+        status = $(cholname(:solve2, TI))(sys, F, B, C_NULL, Ref(X_Handle), C_NULL, Ref(Y_Handle), Ref(E_Handle), getcommon($TI))
+        free!(Y_Handle)
+        free!(E_Handle)
+        @assert !iszero(status)
+        return X
+    end
+
     function spsolve(sys::Integer, F::Factor{Tv, $TI}, B::Sparse{Tv, $TI}) where Tv<:VTypes
         if size(F,1) != size(B,1)
             throw(DimensionMismatch("LHS and RHS should have the same number of rows. " *
@@ -795,7 +823,7 @@ function ssmult(A::Sparse{Tv1, Ti1}, B::Sparse{Tv2, Ti2}, stype::Integer,
     A, B = convert.(Sparse{promote_type(Tv1, Tv2), promote_type(Ti1, Ti2)}, (A, B))
     return ssmult(A, B, stype, values, sorted)
 end
-function horzcat(A::Sparse{Tv1, Ti1}, B::Sparse{Tv2, Ti2}, values::Bool) where 
+function horzcat(A::Sparse{Tv1, Ti1}, B::Sparse{Tv2, Ti2}, values::Bool) where
         {Tv1<:VRealTypes, Tv2<:VRealTypes, Ti1, Ti2}
     A, B = convert.(Sparse{promote_type(Tv1, Tv2), promote_type(Ti1, Ti2)}, (A, B))
     return horzcat(A, B, values)
@@ -809,7 +837,7 @@ function sdmult!(A::Sparse{Tv1, Ti}, transpose::Bool,
     A, X = convert(Sparse{Tv3, Ti}, A), convert(Dense{Tv3}, X)
     return sdmult!(A, transpose, α, β, X, Y)
 end
-function vertcat(A::Sparse{Tv1, Ti1}, B::Sparse{Tv2, Ti2}, values::Bool) where 
+function vertcat(A::Sparse{Tv1, Ti1}, B::Sparse{Tv2, Ti2}, values::Bool) where
         {Tv1<:VRealTypes, Ti1, Tv2<:VRealTypes, Ti2}
     A, B = convert.(Sparse{promote_type(Tv1, Tv2), promote_type(Ti1, Ti2)}, (A, B))
     return vertcat(A, B, values)
@@ -895,7 +923,7 @@ function Dense(A::StridedVecOrMatInclAdjAndTrans)
     return Dense{T}(A)
 end
 # Don't always promote to Float64 now that we have Float32 support.
-Dense(A::StridedVecOrMatInclAdjAndTrans{T}) where 
+Dense(A::StridedVecOrMatInclAdjAndTrans{T}) where
     {T<:Union{Float16, ComplexF16, Float32, ComplexF32}} = Dense{promote_type(T, Float32)}(A)
 
 
@@ -1055,8 +1083,8 @@ Sparse(A::Hermitian{Tv, SparseMatrixCSC{Tv,Ti}}) where {Tv, Ti} =
     Sparse{promote_type(Tv, Float64), Ti <: ITypes ? Ti : promote_type(Ti, Int)}(
         A.data, A.uplo == 'L' ? -1 : 1
     )
-Sparse(A::Hermitian{Tv, SparseMatrixCSC{Tv,Ti}}) where 
-    {Tv<:Union{Float16, Float32, ComplexF32, ComplexF16}, Ti} = 
+Sparse(A::Hermitian{Tv, SparseMatrixCSC{Tv,Ti}}) where
+    {Tv<:Union{Float16, Float32, ComplexF32, ComplexF16}, Ti} =
     Sparse{promote_type(Float32, Tv), Ti <: ITypes ? Ti : promote_type(Ti, Int)}(
         A.data, A.uplo == 'L' ? -1 : 1
     )
@@ -1076,7 +1104,7 @@ function Base.convert(::Type{Sparse{Tnew, Inew}}, A::Sparse{Tv, Ti}) where {Tnew
         a = unsafe_load(typedpointer(A))
         S = allocate_sparse(a.nrow, a.ncol, a.nzmax, Bool(a.sorted), Bool(a.packed), a.stype, Tnew, Inew)
         s = unsafe_load(typedpointer(S))
-        
+
         ap = unsafe_wrap(Array, a.p, (a.ncol + 1,), own = false)
         sp = unsafe_wrap(Array, s.p, (s.ncol + 1,), own = false)
         copyto!(sp, ap)
@@ -1376,7 +1404,7 @@ end
 
 ## Multiplication
 (*)(A::Sparse, B::Sparse) = ssmult(A, B, 0, true, true)
-(*)(A::Sparse, B::Dense) = sdmult!(A, false, 1., 0., B, 
+(*)(A::Sparse, B::Dense) = sdmult!(A, false, 1., 0., B,
     zeros(size(A, 1), size(B, 2), promote_type(eltype(A), eltype(B)))
 )
 (*)(A::Sparse, B::VecOrMat) = (*)(A, Dense(B))
@@ -1413,7 +1441,7 @@ function *(adjA::Adjoint{<:Any,<:Sparse}, B::Sparse)
 end
 
 *(adjA::Adjoint{<:Any,<:Sparse}, B::Dense) = (
-    A = parent(adjA); sdmult!(A, true, 1., 0., B, 
+    A = parent(adjA); sdmult!(A, true, 1., 0., B,
     zeros(size(A, 2), size(B, 2), promote_type(eltype(A), eltype(B))))
 )
 *(adjA::Adjoint{<:Any,<:Sparse}, B::VecOrMat) = adjA * Dense(B)
@@ -1467,7 +1495,7 @@ See also [`cholesky`](@ref).
 
 !!! note
     This method uses the CHOLMOD library from SuiteSparse, which only supports
-    real or complex types in single or double precision. 
+    real or complex types in single or double precision.
     Input matrices not of those element types will
     be converted to these types as appropriate.
 """
@@ -1587,8 +1615,8 @@ true
 
 !!! note
     This method uses the CHOLMOD[^ACM887][^DavisHager2009] library from [SuiteSparse](https://github.com/DrTimothyAldenDavis/SuiteSparse).
-    CHOLMOD only supports real or complex types in single or double precision. 
-    Input matrices not of those element types will be 
+    CHOLMOD only supports real or complex types in single or double precision.
+    Input matrices not of those element types will be
     converted to these types as appropriate.
 
     Many other functions from CHOLMOD are wrapped but not exported from the
@@ -1633,8 +1661,8 @@ have the type tag, it must still be symmetric or Hermitian.
 See also [`ldlt`](@ref).
 
 !!! note
-    This method uses the CHOLMOD library from [SuiteSparse](https://github.com/DrTimothyAldenDavis/SuiteSparse), 
-    which only supports real or complex types in single or double precision. 
+    This method uses the CHOLMOD library from [SuiteSparse](https://github.com/DrTimothyAldenDavis/SuiteSparse),
+    which only supports real or complex types in single or double precision.
     Input matrices not of those element types will
     be converted to these types as appropriate.
 """
@@ -1695,7 +1723,7 @@ it should be a permutation of `1:size(A,1)` giving the ordering to use
 
 !!! note
     This method uses the CHOLMOD[^ACM887][^DavisHager2009] library from [SuiteSparse](https://github.com/DrTimothyAldenDavis/SuiteSparse).
-    CHOLMOD only supports real or complex types in single or double precision. 
+    CHOLMOD only supports real or complex types in single or double precision.
     Input matrices not of those element types will
     be converted to these types as appropriate.
 
@@ -1767,7 +1795,7 @@ See also [`lowrankupdate!`](@ref), [`lowrankdowndate`](@ref), [`lowrankdowndate!
 """
 lowrankupdate(F::Factor{Tv}, V::AbstractArray{Tv2}) where {Tv, Tv2} =
     lowrankupdate!(
-        change_xdtype(F, promote_type(Tv, Tv2)), 
+        change_xdtype(F, promote_type(Tv, Tv2)),
         convert(AbstractArray{promote_type(Tv, Tv2)}, V)
     )
 
@@ -1782,7 +1810,7 @@ See also [`lowrankdowndate!`](@ref), [`lowrankupdate`](@ref), [`lowrankupdate!`]
 """
 lowrankdowndate(F::Factor{Tv}, V::AbstractArray{Tv2}) where {Tv, Tv2} =
 lowrankdowndate!(
-    change_xdtype(F, promote_type(Tv, Tv2)), 
+    change_xdtype(F, promote_type(Tv, Tv2)),
     convert(AbstractArray{promote_type(Tv, Tv2)}, V)
 )
 

From 96669c88fdfd209937d62156da8a0e6b8b961a82 Mon Sep 17 00:00:00 2001
From: Hendrik Ranocha <mail@ranocha.de>
Date: Sat, 27 Jul 2024 15:22:13 +0200
Subject: [PATCH 2/8] implement in-place ldiv! for Cholesky factorization

---
 src/solvers/cholmod.jl | 90 +++++++++++++++++++++++++++++++++++++++---
 test/cholmod.jl        | 11 ++++--
 2 files changed, 92 insertions(+), 9 deletions(-)

diff --git a/src/solvers/cholmod.jl b/src/solvers/cholmod.jl
index 7c640b62..25d45445 100644
--- a/src/solvers/cholmod.jl
+++ b/src/solvers/cholmod.jl
@@ -18,7 +18,7 @@ using LinearAlgebra
 using LinearAlgebra: RealHermSymComplexHerm, AdjOrTrans
 import LinearAlgebra: (\), AdjointFactorization,
                  cholesky, cholesky!, det, diag, ishermitian, isposdef,
-                 issuccess, issymmetric, ldlt, ldlt!, logdet,
+                 issuccess, issymmetric, ldiv!, ldlt, ldlt!, logdet,
                  lowrankdowndate, lowrankdowndate!, lowrankupdate, lowrankupdate!
 
 using SparseArrays
@@ -755,7 +755,7 @@ for TI ∈ IndexTypes
         end
         if size(F, 2) != size(X, 1)
             throw(DimensionMismatch("Factorization and solution should match sizes. " *
-                "Factorization has $(size(F, 1)) columns, but solution has $(size(B, 1)) rows."))
+                "Factorization has $(size(F, 1)) columns, but solution has $(size(X, 1)) rows."))
         end
         if !issuccess(F)
             s = unsafe_load(pointer(F))
@@ -766,9 +766,15 @@ for TI ∈ IndexTypes
             end
         end
         X_Handle = pointer(X)
-        Y_Handle = Ptr{LibSuiteSparse.cholmod_dense_struct}()
-        E_Handle = Ptr{LibSuiteSparse.cholmod_dense_struct}()
-        status = $(cholname(:solve2, TI))(sys, F, B, C_NULL, Ref(X_Handle), C_NULL, Ref(Y_Handle), Ref(E_Handle), getcommon($TI))
+        Y_Handle = Ptr{cholmod_dense_struct}(C_NULL)
+        E_Handle = Ptr{cholmod_dense_struct}(C_NULL)
+        status = $(cholname(:solve2, TI))(
+            sys, F,
+            B, C_NULL,
+            Ref(X_Handle), C_NULL,
+            Ref(Y_Handle),
+            Ref(E_Handle),
+            getcommon($TI))
         free!(Y_Handle)
         free!(E_Handle)
         @assert !iszero(status)
@@ -941,6 +947,19 @@ function Base.convert(::Type{Dense{Tnew}}, A::Dense{T}) where {Tnew, T}
 end
 Base.convert(::Type{Dense{T}}, A::Dense{T}) where T = A
 
+function wrap_dense(x::StridedVecOrMat{T}) where {T <: VTypes}
+    dense_x = cholmod_dense_struct()
+    dense_x.nrow = size(x, 1)
+    dense_x.ncol = size(x, 2)
+    dense_x.nzmax = length(x)
+    dense_x.d = stride(x, 2)
+    dense_x.x = pointer(x)
+    dense_x.z = C_NULL
+    dense_x.xtype = xtyp(eltype(x))
+    dense_x.dtype = dtyp(eltype(x))
+    return dense_x
+end
+
 # This constructor assumes zero based colptr and rowval
 function Sparse(m::Integer, n::Integer,
         colptr0::Vector{Ti}, rowval0::Vector{Ti},
@@ -1933,6 +1952,67 @@ const AbstractSparseVecOrMatInclAdjAndTrans = Union{AbstractSparseVecOrMat, AdjO
     throw(ArgumentError("self-adjoint sparse system solve not implemented for sparse rhs B," *
         " consider to convert B to a dense array"))
 
+# in-place ldiv!
+for TI in IndexTypes
+    @eval function ldiv!(X::Dense{T},
+                         L::Factor{T, $TI},
+                         B::Dense{T}) where {T<:VTypes}
+        solve2(CHOLMOD_A, L, B, X)
+        return X
+    end
+
+    @eval function ldiv!(x::StridedVecOrMat{T},
+                         L::Factor{T, $TI},
+                         b::StridedVecOrMat{T}) where {T<:VTypes}
+        if x === b
+            throw(ArgumentError("output array must not be aliased with input array"))
+        end
+        if size(L, 1) != size(b, 1)
+            throw(DimensionMismatch("Factorization and RHS should have the same number of rows. " *
+                "Factorization has $(size(L, 2)) rows, but RHS has $(size(b, 1)) rows."))
+        end
+        if size(L, 2) != size(x, 1)
+            throw(DimensionMismatch("Factorization and solution should match sizes. " *
+                "Factorization has $(size(L, 1)) columns, but solution has $(size(x, 1)) rows."))
+        end
+        if !issuccess(L)
+            s = unsafe_load(pointer(L))
+            if s.is_ll == 1
+                throw(LinearAlgebra.PosDefException(s.minor))
+            else
+                throw(LinearAlgebra.ZeroPivotException(s.minor))
+            end
+        end
+
+        # Just calling Dense(x) or Dense(b) will allocate new
+        # `cholmod_dense_struct`s in CHOLMOD. Instead, we want to reuse
+        # the existing memory. We can do this by creating new
+        # `cholmod_dense_struct`s and filling them manually.
+        dense_x = wrap_dense(x)
+        dense_b = wrap_dense(b)
+
+        X_Handle = Ptr{cholmod_dense_struct}(pointer_from_objref(dense_x))
+        Y_Handle = Ptr{cholmod_dense_struct}(C_NULL)
+        E_Handle = Ptr{cholmod_dense_struct}(C_NULL)
+        # @info "before solve2" X_Handle Y_Handle E_Handle
+        GC.@preserve dense_x dense_b begin
+            status = $(cholname(:solve2, TI))(
+                CHOLMOD_A, L,
+                Ref(dense_b), C_NULL,
+                Ref(X_Handle), C_NULL,
+                Ref(Y_Handle),
+                Ref(E_Handle),
+                getcommon($TI))
+        end
+        # @info "after solve2" X_Handle Y_Handle E_Handle
+        free!(Y_Handle)
+        free!(E_Handle)
+        @assert !iszero(status)
+
+        return x
+    end
+end
+
 ## Other convenience methods
 function diag(F::Factor{Tv, Ti}) where {Tv, Ti}
     f = unsafe_load(typedpointer(F))
diff --git a/test/cholmod.jl b/test/cholmod.jl
index 108b0ff8..fda0eec6 100644
--- a/test/cholmod.jl
+++ b/test/cholmod.jl
@@ -138,6 +138,9 @@ Random.seed!(123)
     @test CHOLMOD.isvalid(chma)
     @test unsafe_load(pointer(chma)).is_ll == 1    # check that it is in fact an LLt
     @test chma\b ≈ x
+    x2 = zero(x)
+    @inferred ldiv!(x2, chma, b)
+    @test x2 ≈ x
     @test nnz(chma) == 489
     @test nnz(cholesky(A, perm=1:size(A,1))) > nnz(chma)
     @test size(chma) == size(A)
@@ -365,9 +368,9 @@ end
     @test isa(CHOLMOD.eye(3), CHOLMOD.Dense{Float64})
 end
 
-@testset "Core functionality ($elty, $elty2)" for 
-    elty in (Tv, Complex{Tv}), 
-    Tv2 in (Float32, Float64), 
+@testset "Core functionality ($elty, $elty2)" for
+    elty in (Tv, Complex{Tv}),
+    Tv2 in (Float32, Float64),
     elty2 in (Tv2, Complex{Tv2}),
     Ti ∈ itypes
     A1 = sparse(Ti[1:5; 1], Ti[1:5; 2], elty <: Real ? randn(Tv, 6) : complex.(randn(Tv, 6), randn(Tv, 6)))
@@ -972,7 +975,7 @@ end
     f = ones(size(K, 1))
     u = K \ f
     residual = norm(f - K * u) / norm(f)
-    @test residual < 1e-6 
+    @test residual < 1e-6
 end
 
 @testset "wrapped sparse matrices" begin

From add26cc4a153ec0e13311ea32569a2755eaaca41 Mon Sep 17 00:00:00 2001
From: Hendrik Ranocha <mail@ranocha.de>
Date: Sat, 27 Jul 2024 15:23:29 +0200
Subject: [PATCH 3/8] ldiv! with Dense not required

---
 src/solvers/cholmod.jl | 43 ------------------------------------------
 1 file changed, 43 deletions(-)

diff --git a/src/solvers/cholmod.jl b/src/solvers/cholmod.jl
index 25d45445..d85ff2af 100644
--- a/src/solvers/cholmod.jl
+++ b/src/solvers/cholmod.jl
@@ -747,40 +747,6 @@ for TI ∈ IndexTypes
         Dense{Tv}($(cholname(:solve, TI))(sys, F, B, getcommon($TI)))
     end
 
-    # solve the system and store the result in X
-    function solve2(sys::Integer, F::Factor{Tv, $TI}, B::Dense{Tv}, X::Dense{Tv}) where Tv<:VTypes
-        if size(F, 1) != size(B, 1)
-            throw(DimensionMismatch("Factorization and RHS should have the same number of rows. " *
-                "Factorization has $(size(F, 2)) rows, but RHS has $(size(B, 1)) rows."))
-        end
-        if size(F, 2) != size(X, 1)
-            throw(DimensionMismatch("Factorization and solution should match sizes. " *
-                "Factorization has $(size(F, 1)) columns, but solution has $(size(X, 1)) rows."))
-        end
-        if !issuccess(F)
-            s = unsafe_load(pointer(F))
-            if s.is_ll == 1
-                throw(LinearAlgebra.PosDefException(s.minor))
-            else
-                throw(LinearAlgebra.ZeroPivotException(s.minor))
-            end
-        end
-        X_Handle = pointer(X)
-        Y_Handle = Ptr{cholmod_dense_struct}(C_NULL)
-        E_Handle = Ptr{cholmod_dense_struct}(C_NULL)
-        status = $(cholname(:solve2, TI))(
-            sys, F,
-            B, C_NULL,
-            Ref(X_Handle), C_NULL,
-            Ref(Y_Handle),
-            Ref(E_Handle),
-            getcommon($TI))
-        free!(Y_Handle)
-        free!(E_Handle)
-        @assert !iszero(status)
-        return X
-    end
-
     function spsolve(sys::Integer, F::Factor{Tv, $TI}, B::Sparse{Tv, $TI}) where Tv<:VTypes
         if size(F,1) != size(B,1)
             throw(DimensionMismatch("LHS and RHS should have the same number of rows. " *
@@ -1954,13 +1920,6 @@ const AbstractSparseVecOrMatInclAdjAndTrans = Union{AbstractSparseVecOrMat, AdjO
 
 # in-place ldiv!
 for TI in IndexTypes
-    @eval function ldiv!(X::Dense{T},
-                         L::Factor{T, $TI},
-                         B::Dense{T}) where {T<:VTypes}
-        solve2(CHOLMOD_A, L, B, X)
-        return X
-    end
-
     @eval function ldiv!(x::StridedVecOrMat{T},
                          L::Factor{T, $TI},
                          b::StridedVecOrMat{T}) where {T<:VTypes}
@@ -1994,7 +1953,6 @@ for TI in IndexTypes
         X_Handle = Ptr{cholmod_dense_struct}(pointer_from_objref(dense_x))
         Y_Handle = Ptr{cholmod_dense_struct}(C_NULL)
         E_Handle = Ptr{cholmod_dense_struct}(C_NULL)
-        # @info "before solve2" X_Handle Y_Handle E_Handle
         GC.@preserve dense_x dense_b begin
             status = $(cholname(:solve2, TI))(
                 CHOLMOD_A, L,
@@ -2004,7 +1962,6 @@ for TI in IndexTypes
                 Ref(E_Handle),
                 getcommon($TI))
         end
-        # @info "after solve2" X_Handle Y_Handle E_Handle
         free!(Y_Handle)
         free!(E_Handle)
         @assert !iszero(status)

From a6f93fe179a716a841ae769ac678416b0a81781b Mon Sep 17 00:00:00 2001
From: Hendrik Ranocha <mail@ranocha.de>
Date: Sat, 27 Jul 2024 15:42:16 +0200
Subject: [PATCH 4/8] import ldiv! for tests

---
 test/cholmod.jl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/cholmod.jl b/test/cholmod.jl
index fda0eec6..b241e37e 100644
--- a/test/cholmod.jl
+++ b/test/cholmod.jl
@@ -13,7 +13,7 @@ using Random
 using Serialization
 using LinearAlgebra:
     I, cholesky, cholesky!, det, diag, eigmax, ishermitian, isposdef, issuccess,
-    issymmetric, ldlt, ldlt!, logdet, norm, opnorm, Diagonal, Hermitian, Symmetric,
+    issymmetric, ldiv!, ldlt, ldlt!, logdet, norm, opnorm, Diagonal, Hermitian, Symmetric,
     PosDefException, ZeroPivotException, RowMaximum
 using SparseArrays
 using SparseArrays: getcolptr

From 1aae31135284dd0ce7932933e99dd839bbae16c8 Mon Sep 17 00:00:00 2001
From: Hendrik Ranocha <mail@ranocha.de>
Date: Sat, 27 Jul 2024 15:59:40 +0200
Subject: [PATCH 5/8] safer

---
 src/solvers/cholmod.jl | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/solvers/cholmod.jl b/src/solvers/cholmod.jl
index d85ff2af..d77093e3 100644
--- a/src/solvers/cholmod.jl
+++ b/src/solvers/cholmod.jl
@@ -1953,8 +1953,8 @@ for TI in IndexTypes
         X_Handle = Ptr{cholmod_dense_struct}(pointer_from_objref(dense_x))
         Y_Handle = Ptr{cholmod_dense_struct}(C_NULL)
         E_Handle = Ptr{cholmod_dense_struct}(C_NULL)
-        GC.@preserve dense_x dense_b begin
-            status = $(cholname(:solve2, TI))(
+        status = GC.@preserve x dense_x b dense_b begin
+            $(cholname(:solve2, TI))(
                 CHOLMOD_A, L,
                 Ref(dense_b), C_NULL,
                 Ref(X_Handle), C_NULL,
@@ -1962,8 +1962,12 @@ for TI in IndexTypes
                 Ref(E_Handle),
                 getcommon($TI))
         end
-        free!(Y_Handle)
-        free!(E_Handle)
+        if Y_Handle != C_NULL
+            free!(Y_Handle)
+        end
+        if E_Handle != C_NULL
+            free!(E_Handle)
+        end
         @assert !iszero(status)
 
         return x

From bf39182c20760efa337e5ec37494810c8c461287 Mon Sep 17 00:00:00 2001
From: Hendrik Ranocha <mail@ranocha.de>
Date: Sat, 27 Jul 2024 16:29:35 +0200
Subject: [PATCH 6/8] fix Dense case and add more tests

---
 src/solvers/cholmod.jl | 16 ++++++++++++++--
 test/cholmod.jl        | 20 ++++++++++++++++++++
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/src/solvers/cholmod.jl b/src/solvers/cholmod.jl
index d77093e3..fb469d0a 100644
--- a/src/solvers/cholmod.jl
+++ b/src/solvers/cholmod.jl
@@ -1947,8 +1947,20 @@ for TI in IndexTypes
         # `cholmod_dense_struct`s in CHOLMOD. Instead, we want to reuse
         # the existing memory. We can do this by creating new
         # `cholmod_dense_struct`s and filling them manually.
-        dense_x = wrap_dense(x)
-        dense_b = wrap_dense(b)
+        # We need to use a special handling for the case of `Dense`
+        # input arrays since the `pointer` refers to the pointer to the
+        # `cholmod_dense`, not to the array values themselves as for
+        # standard arrays.
+        if x isa Dense
+            dense_x = unsafe_load(pointer(x))
+        else
+            dense_x = wrap_dense(x)
+        end
+        if b isa Dense
+            dense_b = unsafe_load(pointer(b))
+        else
+            dense_b = wrap_dense(b)
+        end
 
         X_Handle = Ptr{cholmod_dense_struct}(pointer_from_objref(dense_x))
         Y_Handle = Ptr{cholmod_dense_struct}(C_NULL)
diff --git a/test/cholmod.jl b/test/cholmod.jl
index b241e37e..30404fcd 100644
--- a/test/cholmod.jl
+++ b/test/cholmod.jl
@@ -284,6 +284,26 @@ end
     end
 end
 
+@testset "ldiv! $Tv $Ti" begin
+    local A, x, x2, b, X, X2, B
+    A = sprand(10, 10, 0.1)
+    A = I + A * A'
+    A = convert(SparseMatrixCSC{Tv,Ti}, A)
+    factor = cholesky(A)
+
+    x = fill(Tv(1), 10)
+    b = A * x
+    x2 = zero(x)
+    @inferred ldiv!(x2, factor, b)
+    @test x2 ≈ x
+
+    X = fill(Tv(1), 10, 5)
+    B = A * X
+    X2 = zero(X)
+    @inferred ldiv!(X2, factor, B)
+    @test X2 ≈ X
+end
+
 end #end for Ti ∈ itypes
 
 for Tv ∈ (Float32, Float64)

From 489756f145ed0e81d8af817ba97c3e31f5499558 Mon Sep 17 00:00:00 2001
From: Hendrik Ranocha <mail@ranocha.de>
Date: Sat, 27 Jul 2024 19:36:55 +0200
Subject: [PATCH 7/8] rearrange wrap_dense_and_ptr

---
 src/solvers/cholmod.jl | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/src/solvers/cholmod.jl b/src/solvers/cholmod.jl
index fb469d0a..20198382 100644
--- a/src/solvers/cholmod.jl
+++ b/src/solvers/cholmod.jl
@@ -913,7 +913,11 @@ function Base.convert(::Type{Dense{Tnew}}, A::Dense{T}) where {Tnew, T}
 end
 Base.convert(::Type{Dense{T}}, A::Dense{T}) where T = A
 
-function wrap_dense(x::StridedVecOrMat{T}) where {T <: VTypes}
+# Just calling Dense(x) or Dense(b) will allocate new
+# `cholmod_dense_struct`s in CHOLMOD. Instead, we want to reuse
+# the existing memory. We can do this by creating new
+# `cholmod_dense_struct`s and filling them manually.
+function wrap_dense_and_ptr(x::StridedVecOrMat{T}) where {T <: VTypes}
     dense_x = cholmod_dense_struct()
     dense_x.nrow = size(x, 1)
     dense_x.ncol = size(x, 2)
@@ -923,7 +927,16 @@ function wrap_dense(x::StridedVecOrMat{T}) where {T <: VTypes}
     dense_x.z = C_NULL
     dense_x.xtype = xtyp(eltype(x))
     dense_x.dtype = dtyp(eltype(x))
-    return dense_x
+    return dense_x, pointer_from_objref(dense_x)
+end
+# We need to use a special handling for the case of `Dense`
+# input arrays since the `pointer` refers to the pointer to the
+# `cholmod_dense`, not to the array values themselves as for
+# standard arrays.
+function wrap_dense_and_ptr(x::Dense{T}) where {T <: VTypes}
+    dense_x_ptr = x.ptr
+    dense_x = unsafe_load(dense_x_ptr)
+    return dense_x, pointer_from_objref(dense_x)
 end
 
 # This constructor assumes zero based colptr and rowval
@@ -1947,22 +1960,10 @@ for TI in IndexTypes
         # `cholmod_dense_struct`s in CHOLMOD. Instead, we want to reuse
         # the existing memory. We can do this by creating new
         # `cholmod_dense_struct`s and filling them manually.
-        # We need to use a special handling for the case of `Dense`
-        # input arrays since the `pointer` refers to the pointer to the
-        # `cholmod_dense`, not to the array values themselves as for
-        # standard arrays.
-        if x isa Dense
-            dense_x = unsafe_load(pointer(x))
-        else
-            dense_x = wrap_dense(x)
-        end
-        if b isa Dense
-            dense_b = unsafe_load(pointer(b))
-        else
-            dense_b = wrap_dense(b)
-        end
+        dense_x, dense_x_ptr = wrap_dense_and_ptr(x)
+        dense_b, dense_b_ptr = wrap_dense_and_ptr(b)
 
-        X_Handle = Ptr{cholmod_dense_struct}(pointer_from_objref(dense_x))
+        X_Handle = Ptr{cholmod_dense_struct}(dense_x_ptr)
         Y_Handle = Ptr{cholmod_dense_struct}(C_NULL)
         E_Handle = Ptr{cholmod_dense_struct}(C_NULL)
         status = GC.@preserve x dense_x b dense_b begin

From fcf34fb0b1ee184106d32c09322a45ade88733bc Mon Sep 17 00:00:00 2001
From: Hendrik Ranocha <mail@ranocha.de>
Date: Sat, 27 Jul 2024 19:43:46 +0200
Subject: [PATCH 8/8] more tests

---
 src/solvers/cholmod.jl |  4 ++++
 test/cholmod.jl        | 11 +++++++++++
 2 files changed, 15 insertions(+)

diff --git a/src/solvers/cholmod.jl b/src/solvers/cholmod.jl
index 20198382..985e80ac 100644
--- a/src/solvers/cholmod.jl
+++ b/src/solvers/cholmod.jl
@@ -1947,6 +1947,10 @@ for TI in IndexTypes
             throw(DimensionMismatch("Factorization and solution should match sizes. " *
                 "Factorization has $(size(L, 1)) columns, but solution has $(size(x, 1)) rows."))
         end
+        if size(x, 2) != size(b, 2)
+            throw(DimensionMismatch("Solution and RHS should have the same number of columns. " *
+                "Solution has $(size(x, 2)) columns, but RHS has $(size(b, 2)) columns."))
+        end
         if !issuccess(L)
             s = unsafe_load(pointer(L))
             if s.is_ll == 1
diff --git a/test/cholmod.jl b/test/cholmod.jl
index 30404fcd..cbcc0ea9 100644
--- a/test/cholmod.jl
+++ b/test/cholmod.jl
@@ -302,6 +302,17 @@ end
     X2 = zero(X)
     @inferred ldiv!(X2, factor, B)
     @test X2 ≈ X
+
+    c = fill(Tv(1), size(x, 1) + 1)
+    C = fill(Tv(1), size(X, 1) + 1, size(X, 2))
+    y = fill(Tv(1), size(x, 1) + 1)
+    Y = fill(Tv(1), size(X, 1) + 1, size(X, 2))
+    @test_throws DimensionMismatch ldiv!(y, factor, b)
+    @test_throws DimensionMismatch ldiv!(Y, factor, B)
+    @test_throws DimensionMismatch ldiv!(x2, factor, c)
+    @test_throws DimensionMismatch ldiv!(X2, factor, C)
+    @test_throws DimensionMismatch ldiv!(X2, factor, b)
+    @test_throws DimensionMismatch ldiv!(x2, factor, B)
 end
 
 end #end for Ti ∈ itypes