[PATCH] libga68: Don't call free on gc-allocated strings
Jose E. Marchesi
jemarch@gnu.org
Thu Jan 29 01:13:31 GMT 2026
Hi Pietro.
Thanks for the patch.
> _libga68_u32_to_u8 called free on the result buffer on error, but the
> buffer is allocated by the GC, so calling free on it is incorrect.
But if LIBGA68_WITH_GC is not defined then _libga68_malloc_leaf used the
regular malloc, right? So the memory needs to be free or we will be
leaking memory..
Wouldn't it be better to make these functions always use
_libga68_malloc_internal and _libga68_malloc_free instead?
>
> Remove the resultbuf parameter from the function since all
> callers use NULL for it.
>
> libga68/ChangeLog:
>
> * ga68-posix.c (_libga68_posixperror): Adjust calls to _libga68_u32_to_u8.
> (_libga68_posixfopen): Likewise.
> (_libga68_posixcreat): Likewise.
> (_libga68_posixgetenv): Likewise.
> (_libga68_posixfputs): Likewise.
> (_libga68_posixfconnect): Likewise.
> * ga68-unistr.c (_libga68_u32_to_u8): Don't call free on result.
> * ga68.h (_libga68_u32_to_u8): Update prototype.
>
> libga68/ChangeLog:
>
> * ga68-posix.c (_libga68_posixperror):
> (_libga68_posixfopen):
> (_libga68_posixcreat):
> (_libga68_posixgetenv):
> (_libga68_posixfputs):
> (_libga68_posixfconnect):
> * ga68-unistr.c (_libga68_u32_to_u8):
> * ga68.h (_libga68_u32_to_u8):
>
> Signed-off-by: Pietro Monteiro <pietro@sociotechnical.xyz>
> ---
> libga68/ga68-posix.c | 13 ++++++-------
> libga68/ga68-unistr.c | 30 ++++++++----------------------
> libga68/ga68.h | 2 +-
> 3 files changed, 15 insertions(+), 30 deletions(-)
>
> diff --git a/libga68/ga68-posix.c b/libga68/ga68-posix.c
> index b6fba202497..1ec5af7595a 100644
> --- a/libga68/ga68-posix.c
> +++ b/libga68/ga68-posix.c
> @@ -57,7 +57,7 @@ void
> _libga68_posixperror (uint32_t *s, size_t len, size_t stride)
> {
> size_t u8len;
> - uint8_t *u8str = _libga68_u32_to_u8 (s, len, stride, NULL, &u8len);
> + uint8_t *u8str = _libga68_u32_to_u8 (s, len, stride, &u8len);
>
> const char *errstr = strerror (_libga68_errno);
> (void) write (2, u8str, u8len);
> @@ -95,8 +95,7 @@ _libga68_posixfopen (const uint32_t *pathname, size_t len, size_t stride,
> int fd;
> int openflags = 0;
> size_t u8len;
> - const uint8_t *u8pathname = _libga68_u32_to_u8 (pathname, len, stride, NULL,
> - &u8len);
> + const uint8_t *u8pathname = _libga68_u32_to_u8 (pathname, len, stride, &u8len);
> char *filepath = (char *) _libga68_malloc_internal (u8len + 1);
> memcpy (filepath, u8pathname, u8len);
> filepath[u8len] = '\0';
> @@ -141,7 +140,7 @@ _libga68_posixcreat (uint32_t *pathname, size_t len, size_t stride,
> uint32_t mode)
> {
> size_t u8len;
> - uint8_t *u8pathname = _libga68_u32_to_u8 (pathname, len, stride, NULL, &u8len);
> + uint8_t *u8pathname = _libga68_u32_to_u8 (pathname, len, stride, &u8len);
> u8pathname[u8len] = '\0';
>
> int res = creat (u8pathname, mode);
> @@ -190,7 +189,7 @@ _libga68_posixgetenv (uint32_t *s, size_t len, size_t stride,
> uint32_t **r, size_t *rlen)
> {
> size_t varlen;
> - char *varname = _libga68_u32_to_u8 (s, len, stride, NULL, &varlen);
> + char *varname = _libga68_u32_to_u8 (s, len, stride, &varlen);
>
> char *var = _libga68_malloc_internal (varlen + 1);
> memcpy (var, varname, varlen);
> @@ -222,7 +221,7 @@ int
> _libga68_posixfputs (int fd, uint32_t *s, size_t len, size_t stride)
> {
> size_t u8len;
> - uint8_t *u8str = _libga68_u32_to_u8 (s, len, stride, NULL, &u8len);
> + uint8_t *u8str = _libga68_u32_to_u8 (s, len, stride, &u8len);
>
> ssize_t ret = write (fd, u8str, u8len);
> _libga68_errno = errno;
> @@ -371,7 +370,7 @@ _libga68_posixfconnect (uint32_t *str, size_t len, size_t stride,
> int port)
> {
> size_t u8len;
> - uint8_t *u8host = _libga68_u32_to_u8 (str, len, stride, NULL, &u8len);
> + uint8_t *u8host = _libga68_u32_to_u8 (str, len, stride, &u8len);
>
> /* Create a stream socket. */
> int fd = socket (AF_INET, SOCK_STREAM, 0);
> diff --git a/libga68/ga68-unistr.c b/libga68/ga68-unistr.c
> index 2a71313c181..9ecfa46bbf1 100644
> --- a/libga68/ga68-unistr.c
> +++ b/libga68/ga68-unistr.c
> @@ -308,31 +308,19 @@ _libga68_u8_uctomb (uint8_t *s, uint32_t uc, ptrdiff_t n)
> /* Convert UCS-4 to UTF-8 */
>
> uint8_t *
> -_libga68_u32_to_u8 (const uint32_t *s, size_t n, size_t stride,
> - uint8_t *resultbuf, size_t *lengthp)
> +_libga68_u32_to_u8 (const uint32_t *s, size_t n, size_t stride, size_t *lengthp)
> {
> const uint32_t *s_end;
> /* Output string accumulator. */
> - uint8_t *result;
> - size_t allocated;
> - size_t length;
> + uint8_t *result = NULL;
> + size_t allocated = 0;
> + size_t length = 0;
>
> stride = stride / sizeof (uint32_t);
> s_end = s + (n * stride);
>
> - if (resultbuf != NULL)
> - {
> - result = resultbuf;
> - allocated = *lengthp;
> - }
> - else
> - {
> - result = NULL;
> - allocated = 0;
> - }
> - length = 0;
> /* Invariants:
> - result is either == resultbuf or == NULL or malloc-allocated.
> + result is either == NULL or gc-allocated.
> If length > 0, then result != NULL. */
>
> while (s < s_end)
> @@ -350,8 +338,6 @@ _libga68_u32_to_u8 (const uint32_t *s, size_t n, size_t stride,
> count = _libga68_u8_uctomb (result + length, uc, allocated - length);
> if (count == -1)
> {
> - if (!(result == resultbuf || result == NULL))
> - free (result);
> errno = EILSEQ;
> return NULL;
> }
> @@ -362,13 +348,13 @@ _libga68_u32_to_u8 (const uint32_t *s, size_t n, size_t stride,
> allocated = (allocated > 0 ? 2 * allocated : 12);
> if (length + 6 > allocated)
> allocated = length + 6;
> - if (result == resultbuf || result == NULL)
> + if (result == NULL)
> memory = (uint8_t *) _libga68_malloc_leaf (allocated * sizeof (uint8_t));
> else
> memory =
> (uint8_t *) _libga68_realloc (result, allocated * sizeof (uint8_t));
>
> - if (result == resultbuf && length > 0)
> + if (result == NULL && length > 0)
> memcpy ((char *) memory, (char *) result,
> length * sizeof (uint8_t));
> result = memory;
> @@ -392,7 +378,7 @@ _libga68_u32_to_u8 (const uint32_t *s, size_t n, size_t stride,
> }
> }
> }
> - else if (result != resultbuf && length < allocated)
> + else if (length < allocated)
> {
> /* Shrink the allocated memory if possible. */
> uint8_t *memory;
> diff --git a/libga68/ga68.h b/libga68/ga68.h
> index 8d1cf20c162..a63efa5b285 100644
> --- a/libga68/ga68.h
> +++ b/libga68/ga68.h
> @@ -118,7 +118,7 @@ int _libga68_u32_cmp2 (const uint32_t *s1, size_t n1, size_t stride1,
> int _libga68_u8_uctomb (uint8_t *s, uint32_t uc, ptrdiff_t n) GA68_HIDDEN;
> int _libga68_u8_mbtouc (uint32_t *puc, const uint8_t *s, size_t n) GA68_HIDDEN;
> uint8_t *_libga68_u32_to_u8 (const uint32_t *s, size_t n, size_t stride,
> - uint8_t *resultbuf, size_t *lengthp) GA68_HIDDEN;
> + size_t *lengthp) GA68_HIDDEN;
> uint32_t *_libga68_u8_to_u32 (const uint8_t *s, size_t n,
> uint32_t *resultbuf, size_t *lengthp);
>
>
> base-commit: 67bb07e67270878f9d3b032f84ee86b970d84b15
More information about the Algol68
mailing list