[Feature request] Moving a known length cstr to a string_t without using memcpy?

[F3llFr0mTh3Sky]

Hello @P-p-H-d, I'd like to know if the following code is valid:

// FIXME Does it works???
string_s *string_create_steal_cstr_ptr(char *data, const size_t len) {
    if (data == NULL) {
        return NULL;
    }

    // It might look like an useless test but it isn't.
    if (data[len - 1] != '\0') {
        return NULL;
    }

    // FIXME Ensure str is correctly allocated on heap.
    string_s *str = (string_s *) malloc(sizeof(struct string_s));

    // FIXME Init the string_t or not???
    //string_init(*str);

    if (str == NULL) {
        // FIXME Proper error management, try using throw catch exception if possible.
        return NULL;
    }

    size_t offset = sizeof(m_str1ng_size_t);

    str->u.heap.alloc[offset-2] = 1;
    str->u.heap.alloc[offset-1] = 31;
    str->u.heap.size = len - 1,
    str->u.heap.ptr = (
        (
            struct {
                long long _n;
                // FIXME Use char *_d, char _d[sizeof(data)] or char _d[len] ???
                //char _d[sizeof(data)];
                //char _d[len];
                char *_d;
            }
        ) {
            ._n=0,
            ._d=data
        }
    )._d;

    return str;
}

In case yes, could you add such a feature to m*lib?

It would allow me to bypass using memcpy and strlen in some use cases of your library string_t structs.

[P-p-H-d]

No, it is not officially supported. But you have the macro STRING_CTE that allows to create but I feel like your code is inspired by it.

What do you want to do? A function like string_init_move but which takes as input an allocated 'char *' (string_init_move_cstr) ? (This could be possible with the constraint that the data pointer shall be properly aligned like a 'long long')

[F3llFr0mTh3Sky]

you have the macro STRING_CTE that allows to create but I feel like your code is inspired by it.

It is indeed. Glad you noticed it. :P

What do you want to do? A function like string_init_move but which takes as input an allocated 'char *' (string_init_move_cstr) ?

Something like that yes. A bit like STRING_CTE but that would allow taking any char *, wether on stack, heap or static instead of only static char [] ones.

(This could be possible with the constraint that the data pointer shall be properly aligned like a 'long long')

So the code I shown you should work and is fine or not?

[P-p-H-d]

First, it uses internal fields which are not supported and may change in the future.

Otherwise to decide if the code is valid or not, I need to have its specification:

• is the given data parameter is correctly aligned?
• the string cannot be reallocated and therefore it can only be used as source pointer.
• who owns the data pointed? The created string_t or the caller?
• how it will be used by the user?

I feel like the proposed prototype cannot be used properly by an user.

[F3llFr0mTh3Sky]

First, it uses internal fields which are not supported and may change in the future.

No clue what to tell you about that. I tried to replicated the working of STRING_CTE the best I can.

Otherwise to decide if the code is valid or not, I need to have its specification:

is the given data parameter is correctly aligned?

Correctly aligned according to what?

the string cannot be reallocated and therefore it can only be used as source pointer.

I don't plan reallocating the string either.

who owns the data pointed? The created string_t or the caller?

I'm not sure? Actually, in my case both this string_t and the data will be freed after being concatenated to another string_t as I described below.
The data might be freed lately is for e.g. it is from a value stored internally by PostgreSQL or a JSON.

how it will be used by the user?

Only for safely concatenating the data to another string_t by using string_cat or string_cats, one that would be initialized the official way.
I don't want to have to use strlen at all when concatenating strings together.

[F3llFr0mTh3Sky]

Here's a sample program I wrote to illustrate my usage of the feature:

int main(void) {
    string_t *str_hello = (string_t *)STRING_CTE("HELLO");
    char world[] = "WORLD";
    size_t len = ArrLen("WORLD");

    string_t *str_world = string_create_steal_cstr_ptr(world, len);

    string_t *str_hello_world = string_create_empty();

    string_cats(
        *str_hello_world,
        *str_hello,
        *str_world
    );

    printf(
        "%s",
        string_get_cstr(*str_hello_world)
    );
}

After running it, it does seems to work correctly with the above function.

[P-p-H-d]

There are several issues:

• STRING_CTE returns a const string because it needs the string to be immutable. Casting into a string_t * breaks the contract. str_hello could be given as output whereas it cannot be used as output, and the compiler cannot detect such errors. Moreover it shall not be freed, whereas the programmer will want to do it and it is illegal.
• there is no free of the strings so there is memory leaks.
• for str_world, some string functions (like string_hash) won't work as their assumptions are not valid. Moreover they should be partially freed (the string_t allocated on the heap, but not the pointed data).

I have some questions:

• why allocating each string_t on the heap?

[F3llFr0mTh3Sky]

There are several issues:

STRING_CTE returns a const string because it needs the string to be immutable. Casting into a string_t * breaks the contract.

Actually, yeah I should just make all my functions parameters const and I won't need to cast them from const string_t * to string_t * anymore.

str_hello could be given as output whereas it cannot be used as output, and the compiler cannot detect such errors.

You meant "could be given as input whereas it cannot be used as input"?

Moreover it shall not be freed, whereas the programmer will want to do it and it is illegal.

That, I know it. I was using another string library like yours before.

there is no free of the strings so there is memory leaks.

I forgot to do it in the sample, but I was doing it like this in my software:

string_clear(*world);
free(world);

string_clear(*hello_world);
free(hello_world);

for str_world, some string functions (like string_hash) won't work as their assumptions are not valid. Moreover they should be partially freed (the string_t allocated on the heap, but not the pointed data).

ok? Is there a way to fix this behaviour?

I have some questions:
why allocating each string_t on the heap?

Actually, the string_t seems to be pretty big objects, and I thought it would be faster to just pass string_t * as function parameters and return instead of string_t. Furthermore, I'm generally using multithreading and sharing strings across threads.

[P-p-H-d]

Actually, the string_t seems to be pretty big objects, and I thought it would be faster to just pass string_t * as function parameters and return instead of string_t. Furthermore, I'm generally using multithreading and sharing strings across threads.

string_t are just double the size of a pointer, so there are not so big. Moreover they are automatically passed as pointer for function parameters, so there is no performance issue. I think that using string_t * will only make it slower.

If you're using string_t in multithreading environments, you may look at the shared-ptr container (it can transform a string_t into a shared string_t, providing proper locks).

[F3llFr0mTh3Sky]

Moreover they are automatically passed as pointer for function parameters

How?

[P-p-H-d]

They are defined as:

typedef struct string_s string_t[1];
and arrays decade to pointer when being passed as function argument.

[F3llFr0mTh3Sky]

So what I'm passing to my functions are string_s **?

ffs that's kinda dumb of me.

[F3llFr0mTh3Sky]

I fixed the string_create_steal_cstr_ptr function in my first post.
So far, it is working very well except for one use case,
When I'm creating a string_s * using string_create_steal_cstr_ptr by "stealing" a char * from a yyjson_val * and I then try to use this string_s * as the key of an entry of an M*Lib dict, I'm getting this error:

Main: /usr/include/m-lib/m-core.h:3382:
uint64_t m_core_hash(const void *, size_t):
Assertion `(( (uintptr_t)p & (sizeof(uint64_t)-1) ) == 0) ||
(length <= sizeof(uint32_t))' failed.

It is weird since there's 99% chance the length of the string_s * is initialized with the right value.
By that I mean that yyjson_get_len doesn't include the +1 for the ending "\0" character.

However, if I used a copy of the string_s * created with my string_create_from_cstr function as the key of an entry of an M*Lib dict, it works ... and this function is the same as the other one, except it does a full copy of the char * on heap.

The function I'm using for converting an yyjson string to a string_s * is this one:

string_s *string_from_json_string(yyjson_val *json_str) {
    size_t length = yyjson_get_len(json_str);

    string_s *str = string_create_steal_cstr_ptr(
        (char *)yyjson_get_str(json_str),
        length + 1
    );

    return str;
}

[P-p-H-d]

The issue is that the char* is not correctly aligned, which is to expected.
That's why a copy on the heap works.

[F3llFr0mTh3Sky]

I see. So, is there another way to get a well aligned 'char *' than doing a full copy of it and its content on heap?

If there's none it's fine, I'll just keep using the copy on heap.

[F3llFr0mTh3Sky]

I just discovered another issue:
By trying to use string_replace to replace some characters in a string_s * created like that:

const char blah[] = "BLAH";

#define ArrLen(Arr) ((&Arr)[1] - Arr)

size_t len = ArrLen(_bleh);

char *bleh = (char *)malloc(len * sizeof(char));

memcpy(bleh, blah, len);

string_t *str_bleh = string_create_steal_cstr_ptr(bleh, len);

string_replace(str_bleh, STRING_CTE("BLAH"), STRING_CTE("BLEH BLEH BLEH MEUH MEUH MEUH!"));

I'll get the following error:

==1781==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7375c7cfdcb9 at pc 0x589df2ee2eb9 bp 0x7245ae7377d0 sp 0x7245ae736f80
WRITE of size 245 at 0x7375c7cfdcb9 thread T39
    #0 0x589df2ee2eb8 in memmove (/home/calico/Workspaces/CLionProjects/TEST/build/linux/x86_64/debug/Main+0x247eb8)
    #1 0x589df2f37e1c in m_string_replace_cstr /usr/include/m-lib/m-string.h:868:7
    #2 0x589df2f324a9 in m_string_replace /usr/include/m-lib/m-string.h:886:10
[...]

I thought that string_replace would reallocate the char * of the string_s * even though the capacity of the string wasn't set properly? Am I missing a call to string_init?