On Fri, Feb 28, 2020 at 03:13:36PM +0100, Tim Duesterhus wrote:
> istdup() performs the equivalent of strdup() on a `struct ist`.
> ---
> include/common/ist.h | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/include/common/ist.h b/include/common/ist.h
> index 4d75d8607..9614f957d 100644
> --- a/include/common/ist.h
> +++ b/include/common/ist.h
> @@ -420,6 +420,23 @@ static inline ssize_t istscpy(struct ist *dst, const
> struct ist src, size_t coun
> return -1;
> }
>
> +/* This function performs the equivalent of strdup() on the given <src>.
> + * If this function fails to allocate memory an `ist` with `.ptr = NULL` and
> `.len = 0`
> + * is returned.
Here you can simply state that it returns IST_NULL based on the previous
patch, thus by definition the result may be tested using ist_test().
> + */
> +static inline struct ist istdup(const struct ist src)
> +{
> + const size_t alloc_size = src.len;
> + const char *alloc = malloc(alloc_size);
> + struct ist dst = ist2(alloc, 0);
Be careful, it's not correct to place a const above, since the area will
be writable. I guess it passes thanks to the ist2() function that purposely
casts a const to a var in order to be usable both for constants and variables.
> +
> + if (dst.ptr != NULL) {
> + istcpy(&dst, src, alloc_size);
> + }
I'm seeing some caveats here, as malloc(0) may return NULL, resulting in
a failed copy of an empty string. So probably we'll need to force the value
to be at least one. I think it can also make sense to consider that a NULL
input results in a NULL output, this can drop a large number of tests.
We'd need to have the equivalent istfree() to perform the opposite operation.
However I'm having a concern now. The ist "lib" was purposely created as
a freestanding code that can be used anywhere, even in a kernel or a boot
loader, and now we're making it depend on stdlib one malloc implementation.
I think we need to place this into a separate header which includes ist.h
(just as we did for istbuf). Then ist.h will continue to provide the raw
operations.
Well, I can propose you something in order not to add too much burden for
now. Let's just add this at the top of the file:
#ifndef IST_FREESTANDING
#include <stdlib.h>
#endif
And place your new alloc/free functions into another #ifndef IST_FREESTANDING
block at the bottom. Once we figure how to cleanly split this later and how
to rename the different parts, it will be easy to move.
And actually, having this ifdef could also be an elegant solution to decide
whether to use strlen()/memcpy() etc instead of the open-coded loops in some
places (after using hints like __builtin_constant_p() to let the compiler
continue to compute lengths or even outputs without emitting code for them).
Thanks!
Willy
