Merge pull request #436 from UriKH/fix/mpoly-str-leak

Fix memory leak in str() of fmpz_mpoly, fmpq_mpoly, fmpz_mod_mpoly, nmod_mpoly

Author: oscarbenjamin

README.md

@@ -170,9 +170,15 @@ Contributors (0.10.0):
 - Vitaly Magerya (VM)
 - Oscar Benjamin (OB)
 - Ilia Gradina (IG)
+- Uri K.H. (UKH)
 
 Changes (0.10.0):
 
+- [gh-435](https://github.com/flintlib/python-flint/issues/435),
+  Fix a memory leak in `str()` (and so in `repr()`, which uses it unless
+  `flint.ctx.pretty` is `False`) of `fmpz_mpoly`, `fmpq_mpoly`,
+  `fmpz_mod_mpoly` and `nmod_mpoly`: the string returned by FLINT was
+  copied into a Python `bytes` object but never freed. (UKH)
 - [gh-433](https://github.com/flintlib/python-flint/pull/433),
   Define the input type aliases in the type stubs (such as `iarb` and
   `ifmpz`) after the classes they reference, so that the ty type checker

src/flint/types/fmpq_mpoly.pyx

@@ -10,6 +10,7 @@ from flint.flint_base.flint_base cimport (
 )
 
 from flint.utils.typecheck cimport typecheck
+from flint.flintlib.functions.flint cimport flint_free
 from flint.utils.flint_exceptions import DomainError, IncompatibleContextError
 
 from flint.types.fmpq cimport fmpq, any_as_fmpq
@@ -760,9 +761,13 @@ cdef class fmpq_mpoly(flint_mpoly):
         return f"{self.ctx}.from_dict({self.to_dict()})"
 
     def str(self):
-        cdef bytes s = <bytes> fmpq_mpoly_get_str_pretty(self.val, self.ctx.c_names, self.ctx.val)
+        cdef char * cs = fmpq_mpoly_get_str_pretty(self.val, self.ctx.c_names, self.ctx.val)
+        try:
+            s = (<bytes> cs).decode()
+        finally:
+            flint_free(cs)  # FLINT malloc'ed the string; the bytes cast copies it
         # res = s.decode().replace("+", " + ").replace("-", " - ")
-        res = s.decode()
+        res = s
         if res.startswith("- "):
             res = "-" + res[3:]
         return res

src/flint/types/fmpz_mod_mpoly.pyx

@@ -10,6 +10,7 @@ from flint.flint_base.flint_base cimport (
 )
 
 from flint.utils.typecheck cimport typecheck
+from flint.flintlib.functions.flint cimport flint_free
 from flint.utils.flint_exceptions import DomainError, IncompatibleContextError
 
 from flint.types.fmpz cimport any_as_fmpz, fmpz
@@ -746,8 +747,12 @@ cdef class fmpz_mod_mpoly(flint_mpoly):
         return f"{self.ctx}.from_dict({self.to_dict()})"
 
     def str(self):
-        cdef bytes s = <bytes> fmpz_mod_mpoly_get_str_pretty(self.val, self.ctx.c_names, self.ctx.val)
-        res = s.decode().replace("+", " + ").replace("-", " - ")
+        cdef char * cs = fmpz_mod_mpoly_get_str_pretty(self.val, self.ctx.c_names, self.ctx.val)
+        try:
+            s = (<bytes> cs).decode()
+        finally:
+            flint_free(cs)  # FLINT malloc'ed the string; the bytes cast copies it
+        res = s.replace("+", " + ").replace("-", " - ")
         if res.startswith(" - "):
             res = "-" + res[3:]
         return res

src/flint/types/fmpz_mpoly.pyx

@@ -10,6 +10,7 @@ from flint.flint_base.flint_base cimport (
 )
 
 from flint.utils.typecheck cimport typecheck
+from flint.flintlib.functions.flint cimport flint_free
 from flint.utils.flint_exceptions import DomainError, IncompatibleContextError
 
 from flint.types.fmpz cimport any_as_fmpz, fmpz
@@ -734,8 +735,12 @@ cdef class fmpz_mpoly(flint_mpoly):
         return f"{self.ctx}.from_dict({self.to_dict()})"
 
     def str(self):
-        cdef bytes s = <bytes> fmpz_mpoly_get_str_pretty(self.val, self.ctx.c_names, self.ctx.val)
-        res = s.decode().replace("+", " + ").replace("-", " - ")
+        cdef char * cs = fmpz_mpoly_get_str_pretty(self.val, self.ctx.c_names, self.ctx.val)
+        try:
+            s = (<bytes> cs).decode()
+        finally:
+            flint_free(cs)  # FLINT malloc'ed the string; the bytes cast copies it
+        res = s.replace("+", " + ").replace("-", " - ")
         if res.startswith(" - "):
             res = "-" + res[3:]
         return res

src/flint/types/nmod_mpoly.pyx

@@ -8,6 +8,7 @@ from flint.flint_base.flint_base cimport (
 )
 
 from flint.utils.typecheck cimport typecheck
+from flint.flintlib.functions.flint cimport flint_free
 from flint.utils.flint_exceptions import DomainError, IncompatibleContextError
 
 from flint.types.fmpz cimport fmpz
@@ -720,8 +721,12 @@ cdef class nmod_mpoly(flint_mpoly):
         return f"{self.ctx}.from_dict({self.to_dict()})"
 
     def str(self):
-        cdef bytes s = <bytes> nmod_mpoly_get_str_pretty(self.val, self.ctx.c_names, self.ctx.val)
-        res = s.decode().replace("+", " + ").replace("-", " - ")
+        cdef char * cs = nmod_mpoly_get_str_pretty(self.val, self.ctx.c_names, self.ctx.val)
+        try:
+            s = (<bytes> cs).decode()
+        finally:
+            flint_free(cs)  # FLINT malloc'ed the string; the bytes cast copies it
+        res = s.replace("+", " + ").replace("-", " - ")
         if res.startswith(" - "):
             res = "-" + res[3:]
         return res