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 <[email protected]> > --- > 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
