Hi Pietro.

OK. Thanks!

> This is a v2 of libga68: Don't call free on gc-allocated strings, but
> that commit message is incorrect now.
>
> Since only posix prelude functions use the conversion function, and they
> add a trailling null and pass the result to functions that work on char*,
> I decided to add the trailling null and return a char* anyway.
> -- >8 --
> _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.
>
> Instead of using the GC, use the internal malloc because this is an
> internal function.
>
> Remove the resultbuf parameter from the _libga68_u32_to_u8 since all
> callers used NULL for it.
>
> Last but not least, return a null-terminated char* from
> _libga68_u32_to_u8.
>
> libga68/ChangeLog:
>
>       * ga68-alloc.c (_libga68_realloc_internal): New function.
>       * 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): Use internal allocator.
>       * ga68.h (_libga68_realloc_internal): New prototype.
>       (_libga68_u32_to_u8): Update prototype.
>
> Signed-off-by: Pietro Monteiro <[email protected]>
> ---
>  libga68/ga68-alloc.c  |  9 ++++++
>  libga68/ga68-posix.c  | 36 +++++++++------------
>  libga68/ga68-unistr.c | 73 ++++++++++++++++++-------------------------
>  libga68/ga68.h        |  5 +--
>  4 files changed, 57 insertions(+), 66 deletions(-)
>
> diff --git a/libga68/ga68-alloc.c b/libga68/ga68-alloc.c
> index df8a8956c52..1edc148db69 100644
> --- a/libga68/ga68-alloc.c
> +++ b/libga68/ga68-alloc.c
> @@ -42,6 +42,15 @@ _libga68_malloc_internal (size_t size)
>    return res;
>  }
>  
> +void *
> +_libga68_realloc_internal (void *ptr, size_t size)
> +{
> +  void *res = (void *) realloc (ptr, size);
> +  if (!res)
> +    _libga68_abort ("Virtual memory exhausted\n");
> +  return res;
> +}
> +
>  #if LIBGA68_WITH_GC
>  #include <gc/gc.h>
>  
> diff --git a/libga68/ga68-posix.c b/libga68/ga68-posix.c
> index b6fba202497..ef137933774 100644
> --- a/libga68/ga68-posix.c
> +++ b/libga68/ga68-posix.c
> @@ -57,13 +57,14 @@ 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);
>    (void) write (2, ": ", 2);
>    (void) write (2, errstr, strlen (errstr));
>    (void) write (2, "\n", 1);
> +  _libga68_free_internal (u8str);
>  }
>  
>  uint32_t *
> @@ -95,11 +96,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);
> -  char *filepath = (char *) _libga68_malloc_internal (u8len + 1);
> -  memcpy (filepath, u8pathname, u8len);
> -  filepath[u8len] = '\0';
> +  const char *filepath = _libga68_u32_to_u8 (pathname, len, stride, &u8len);
>  
>    /* Default mode: try read-write initially.
>       If that fails, then try read-only.
> @@ -141,11 +138,11 @@ _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);
> -  u8pathname[u8len] = '\0';
> +  const char *filepath = _libga68_u32_to_u8 (pathname, len, stride, &u8len);
>  
> -  int res = creat (u8pathname, mode);
> +  int res = creat (filepath, mode);
>    _libga68_errno = errno;
> +  _libga68_free_internal (filepath);
>    return res;
>  }
>  
> @@ -189,14 +186,11 @@ void
>  _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);
> +  size_t u8len;
> +  const char *varname = _libga68_u32_to_u8 (s, len, stride, &u8len);
>  
> -  char *var = _libga68_malloc_internal (varlen + 1);
> -  memcpy (var, varname, varlen);
> -  var[varlen] = '\0';
> -  char *val = getenv (var);
> -  _libga68_free_internal (var);
> +  char *val = getenv (varname);
> +  _libga68_free_internal (varname);
>  
>    if (val == NULL)
>      {
> @@ -222,10 +216,11 @@ 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);
> +  const char *u8str = _libga68_u32_to_u8 (s, len, stride, &u8len);
>  
>    ssize_t ret = write (fd, u8str, u8len);
>    _libga68_errno = errno;
> +  _libga68_free_internal (u8str);
>    if (ret == -1)
>      return 0;
>    else
> @@ -371,7 +366,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);
> +  const char *host = _libga68_u32_to_u8 (str, len, stride, &u8len);
>  
>    /* Create a stream socket.  */
>    int fd = socket (AF_INET, SOCK_STREAM, 0);
> @@ -380,9 +375,6 @@ _libga68_posixfconnect (uint32_t *str, size_t len, size_t 
> stride,
>      goto error;
>  
>    /* Lookup the specified host.  */
> -  char *host = _libga68_malloc_internal (u8len + 1);
> -  memcpy (host, u8host, u8len);
> -  host[u8len] = '\0';
>    struct hostent *server = gethostbyname (host);
>    if (server == NULL)
>      {
> @@ -409,8 +401,8 @@ _libga68_posixfconnect (uint32_t *str, size_t len, size_t 
> stride,
>  
>   close_fd_and_error:
>    close (fd);
> - error:
>    _libga68_free_internal (host);
> + error:
>    return -1;
>  }
>  
> diff --git a/libga68/ga68-unistr.c b/libga68/ga68-unistr.c
> index 2a71313c181..e37bd8c852d 100644
> --- a/libga68/ga68-unistr.c
> +++ b/libga68/ga68-unistr.c
> @@ -305,34 +305,26 @@ _libga68_u8_uctomb (uint8_t *s, uint32_t uc, ptrdiff_t 
> n)
>    return -2;
>  }
>  
> -/* 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)
> +/* Convert S of size N with stride STRIDE UCS-4 to UTF-8.
> +   Returns a pointer to the converted string with a traiiling '\0'.
> +   The string length, not counting the trailling '\0', is returned on 
> LENGTHP.
> +   Returns NULL on error.
> +   Callers *must* call _libga68_free_internal on a non-null result.  */
> +
> +char *
> +_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 allocated by the internal malloc.
>       If length > 0, then result != NULL.  */
>  
>    while (s < s_end)
> @@ -350,8 +342,8 @@ _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);
> +       if (result != NULL)
> +         _libga68_free_internal (result);
>            errno = EILSEQ;
>            return NULL;
>          }
> @@ -362,15 +354,12 @@ _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)
> -         memory = (uint8_t *) _libga68_malloc_leaf (allocated * sizeof 
> (uint8_t));
> +       if (result == NULL)
> +         memory = (uint8_t *) _libga68_malloc_internal (allocated * sizeof 
> (uint8_t));
>            else
>           memory =
> -           (uint8_t *) _libga68_realloc (result, allocated * sizeof 
> (uint8_t));
> +           (uint8_t *) _libga68_realloc_internal (result, allocated * sizeof 
> (uint8_t));
>  
> -          if (result == resultbuf && length > 0)
> -            memcpy ((char *) memory, (char *) result,
> -                    length * sizeof (uint8_t));
>            result = memory;
>            count = _libga68_u8_uctomb (result + length, uc, allocated - 
> length);
>            if (count < 0)
> @@ -384,26 +373,26 @@ _libga68_u32_to_u8 (const uint32_t *s, size_t n, size_t 
> stride,
>        if (result == NULL)
>          {
>            /* Return a non-NULL value.  NULL means error.  */
> -       result = (uint8_t *) _libga68_malloc_leaf (1);
> -          if (result == NULL)
> -            {
> -              errno = ENOMEM;
> -              return NULL;
> -            }
> +       result = (uint8_t *) _libga68_malloc_internal (1);
>          }
>      }
> -  else if (result != resultbuf && length < allocated)
> +  else
>      {
> -      /* Shrink the allocated memory if possible.  */
> -      uint8_t *memory;
> -
> -      memory = (uint8_t *) _libga68_realloc_unchecked (result, length * 
> sizeof (uint8_t));
> -      if (memory != NULL)
> -        result = memory;
> +      if (length + 1 != allocated)
> +     {
> +       /* Resize the allocated memory to fit the string plus the trailling 
> NULL.  */
> +       uint8_t *memory;
> +
> +       memory =
> +         (uint8_t *) _libga68_realloc_internal (result,
> +                                                (length  +1) * sizeof 
> (uint8_t));
> +       result = memory;
> +     }
> +      result[length] = 0x0;
>      }
>  
>    *lengthp = length;
> -  return result;
> +  return (char *) result;
>  }
>  
>  /* Used by ga68_u8_to_u32 below.  */
> diff --git a/libga68/ga68.h b/libga68/ga68.h
> index 8d1cf20c162..316a9e318d5 100644
> --- a/libga68/ga68.h
> +++ b/libga68/ga68.h
> @@ -73,6 +73,7 @@ void *_libga68_malloc (size_t size);
>  void *_libga68_malloc_leaf (size_t size);
>  void *_libga68_malloc_internal (size_t size) GA68_HIDDEN;
>  void *_libga68_realloc (void *ptr, size_t size) GA68_HIDDEN;
> +void *_libga68_realloc_internal (void *ptr, size_t size) GA68_HIDDEN;
>  void *_libga68_realloc_unchecked (void *ptr, size_t size) GA68_HIDDEN;
>  void _libga68_free_internal (void *ptr) GA68_HIDDEN;
>  
> @@ -117,8 +118,8 @@ int _libga68_u32_cmp2 (const uint32_t *s1, size_t n1, 
> size_t stride1,
>                      const uint32_t *s2, size_t n2, size_t stride2);
>  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;
> +char *_libga68_u32_to_u8 (const uint32_t *s, size_t n, size_t stride,
> +                       size_t *lengthp) GA68_HIDDEN;
>  uint32_t *_libga68_u8_to_u32 (const uint8_t *s, size_t n,
>                             uint32_t *resultbuf, size_t *lengthp);

Reply via email to