bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: xmemdup0


From: Bruno Haible
Subject: Re: xmemdup0
Date: Sat, 10 May 2008 00:13:20 +0200
User-agent: KMail/1.5.4

Eric Blake wrote:
> Would anyone else be interested in adding xmemdup0 to the xalloc module?

3 comments:

----------------------------------------------------------------------------

I would prefer if you could move it to a module of its own.

We have two data types:
  (M) memory with arbitrary contents, where the start address and the length
      are given,
  (S) memory that is terminated with a NUL byte and otherwise does not contain
      NULs; here only the start address is usually given.

xmemdup0 is a function to convert from (M) to (S) in certain situations.
I don't deny that such a function can occasionally be useful. But it can
also easily be the source of several kinds of bugs:

  Bug 1: What if the memory contents contains a NUL? You do nothing. No
         conversion of a NUL to a backslashed syntax or other escaping?
         Nothing? It would be better to call abort() in this case, then.

  Bug 2: Off-by-one errors. They are also latent when you convert from
         (S) to (M), but here as well.

xalloc is a module that is very widely used. Its consumers should be able
to trust that they won't fall into pitfalls. And since a conversion from
(M) to (S) is rare anyway, I'd suggest to move it to a separate module.

----------------------------------------------------------------------------

When I look where this function could be useful in gettext, I find

  - 19 places where the task is to extract a substring from a string (S).
    Here it is guaranteed that there is no embedded NUL. The function
    does not need to check it; it could be called
        char * xsubstring (char *ptr, size_t len);

  - 9 places where the task is to transform some memory (M) into (S).
    Here the name xmemdup0 is appropriate (or xmemtostr ?), and it
    should better verify that the memory does not contain NUL bytes.
    (Better abort() than produce wrong results.)

So I vote for adding two different functions.

xsubstring should also go to a different module than 'xalloc'. Otherwise
xalloc slowly becomes a module of string manipulations. Let it remain a
module which deals with memory allocation.

----------------------------------------------------------------------------

>    In C++, the return type matches PTR, with any const removed.  */
>
> +template <typename T> inline T *
> +xmemdup0 (T const *p, size_t s)
> +{
> +  return (T *) xmemdup0 ((void const *) p, s);
> +}

This makes no sense to me: If T is, say, 'unsigned int', you are adding
a single-byte '\0' and then casting the pointer to (unsigned int *). But
the last, added 'unsigned int' word is most likely not 0; it is not even
correctly allocated.

The result type should be 'char *', in C++ and also in C. 'unsigned char *'
is not a useful return type, because users want a result in data type (S),
in particular they want strlen() to be applicable to it.

Bruno





reply via email to

[Prev in Thread] Current Thread [Next in Thread]