search

vapor-community / postgresql

Robust PostgreSQL interface for Swift
MIT License
131 stars 33 forks source link

Fixes memory leak. #67

Closed kzaher closed 6 years ago

kzaher commented 6 years ago

This is one way how one could solve #66.

It breaks public interface, but I would argue that public interface had a design flaw in the first place because it was not clear who owns the passed buffer and who is responsible for cleaning up memory.

That was the actual cause for the bug in the first place.

I would suggest changing that interface to be more explicit, but if backwards compatibility is wanted, then there are other solutions.

codecov[bot] commented 6 years ago

Codecov Report

Merging #67 into master will increase coverage by 0.44%. The diff coverage is 93.1%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #67      +/-   ##
==========================================
+ Coverage   79.59%   80.03%   +0.44%     
==========================================
  Files          10       10              
  Lines        1132     1137       +5     
==========================================
+ Hits          901      910       +9     
+ Misses        231      227       -4
Impacted Files Coverage Δ
Sources/PostgreSQL/Result.swift 67.85% <100%> (+2.63%) :arrow_up:
Sources/PostgreSQL/Bind/Bind.swift 78.28% <92.85%> (+1.04%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update abba023...60f6b8c. Read the comment docs.

vzsg commented 6 years ago

Hi.

Thank you for the fix, this is indeed a serious issue and a neat patch.

While technically this is indeed a change in the public API in theory, users of the library aren't supposed to use the Bind class directly. Most notably, the postgresql-driver module is completely unaffected by this change, so I'm in favor of just tagging and releasing this as 2.1.2 officially too.