Skip to content

Changed internal string to support \0 in the middle - #108

Closed
TheBeef wants to merge 3 commits into
paladin-t:masterfrom
TheBeef:master
Closed

Changed internal string to support \0 in the middle#108
TheBeef wants to merge 3 commits into
paladin-t:masterfrom
TheBeef:master

Conversation

@TheBeef

@TheBeef TheBeef commented May 6, 2026

Copy link
Copy Markdown

I've decided to go with my-basic for my built in scripting language in WhippyTerm (https://github.com/TheBeef/WhippyTerm). It's well written with a good interface easy to compile in. Great work :)

I need support for working with 0's in strings. I need to be able to do things like FOR R=0 TO 255 : SEND chr(R) : NEXT
The build in string was a C-String so of course it did not support it.

This patch changes the internal string to a string with length. It also includes a trailing \0 on the string to make it work as a C-String as well.

I have added string_z as a type as well as providing compatibility functions (under the old name) and union access. This means that old code should continue to work, but if someone wants \0 supporting string they would need to change their code to use the _z version.

@paladin-t

paladin-t commented May 11, 2026

Copy link
Copy Markdown
Owner

Being able to use \0 inside MY-BASIC strings is a cool idea. I've read through your PR and it's a completely usable enhancement. Here is some feedback on the PR:

  1. *string_z doesn't feel like a good name to me, because it strongly suggests "zero-terminated string".
  2. In some scenarios memory compactness is more important than support for \0 inside strings. With mb_string_t, mb_value_u and similar data structures typically grow by 4-8 bytes on a common configuration. I'd rather see a macro like MB_ENABLE_SIZED_STRING that can completely enable or disable this new feature.
  3. There is a potential bug with Unicode handling (see the code snippet below).
  4. Finally, and more importantly than the points above, I'm deciding not to merge this PR because it's not the simplest implementation. See the "Suggestion" section below for the reasons and my proposed alternative.

Potential bug with UTF‑8 characters longer than 1 byte:

static int _std_len(mb_interpreter_t* s, void** l) {
    ...
    if((os & MB_MS_DONE) == MB_MS_NONE) {
        switch(arg.type) {
        case MB_DT_STRING:
#ifdef MB_ENABLE_UNICODE
            if(arg.value.string_z.data) {
                size_t code_points = (size_t)mb_uu_strlen(arg.value.string_z.data);
                size_t byte_len = arg.value.string_z.length;
                // When a UTF-8 string contains characters > 1 byte,
                // `code_points` will always be less than `byte_len`,
                // so this can never correctly compute the number of UTF-8 characters.
                _mb_check_mark_exit(mb_push_int(s, l, (int_t)(byte_len > code_points ? byte_len : code_points)), result, _exit);
            } else {
                _mb_check_mark_exit(mb_push_int(s, l, 0), result, _exit);
            }
#else /* MB_ENABLE_UNICODE */
            _mb_check_mark_exit(mb_push_int(s, l, (int_t)arg.value.string.length), result, _exit);
#endif /* MB_ENABLE_UNICODE */
        ...

Suggestion:

With the default configuration, MY-BASIC enables MB_ENABLE_ALLOC_STAT. This feature stores the allocation size in a header before each internal memory allocation, for example in the following functions:

static char* mb_strdup(const char* p, size_t s) {
#ifdef MB_ENABLE_ALLOC_STAT // Enabled by default.
    if(!s) {
        s = _MB_READ_MEM_TAG_SIZE(p); // Read length from allocation header directly.
    }
    return mb_memdup(p, (unsigned)s);
#else /* MB_ENABLE_ALLOC_STAT */
    ...
#endif /* MB_ENABLE_ALLOC_STAT */
}

static void* mb_malloc(size_t s) {
    char* ret = 0;
    size_t rs = s;

#ifdef MB_ENABLE_ALLOC_STAT
    if(!_MB_CHECK_MEM_TAG_SIZE(size_t, s))
        return 0;
    rs += _MB_MEM_TAG_SIZE;
#endif /* MB_ENABLE_ALLOC_STAT */
    if(_mb_allocate_func)
        ret = _mb_allocate_func((unsigned)rs);
    else
        ret = (char*)malloc(rs);
    mb_assert(ret);
#ifdef MB_ENABLE_ALLOC_STAT
    _mb_allocated += s;
    ret += _MB_MEM_TAG_SIZE;
    _MB_WRITE_MEM_TAG_SIZE(ret, s);
#endif /* MB_ENABLE_ALLOC_STAT */

    return (void*)ret;
}

That means MY-BASIC already uses a Pascal-style length-prefixed string for internal allocations. I suggest that in your project you call mb_set_memory_manager to provide your own memory allocator so that both internal and external code use the same allocation/free strategy, and when calling mb_push_string you store the length (including any middle \0) in the allocation header. This approach can be achieved with zero or few modifications (usages of strlen) to MY-BASIC itself. If you further want LEN and similar functions to support \0 holes, you only need to adjust the corresponding statement implementations or implement custom versions of those functions.

Given all of the above, I'm going to close this PR for now. Of course I might have missed something, so further discussion is welcome.

@paladin-t paladin-t closed this May 11, 2026
paladin-t added a commit that referenced this pull request May 11, 2026
…tring) value is managed when pushing it

+Added an mb_set_string_measurer function to measure string length
*Improved string concat
@paladin-t

paladin-t commented May 11, 2026

Copy link
Copy Markdown
Owner

I've updated some of the string-related code, and added new mb_push_managed_value, mb_set_string_measurer functions. Now we can use the code as follows to support \0 in the middle of a string. Note that len and other string APIs behave as before, so consider making new version of these functions if counting \0 in the middle is needed.

// Memory allocator, used to allocate string etc by the core and external code commonly.
static char* my_malloc(unsigned s) {
	char* result = (char*)malloc(s);

	return result;
}

static void my_free(char* p) {
	free(p);
}

// The following macros reads/writes the size of allocations.
#define _MB_CHECK_MEM_TAG_SIZE(y, s) ((y)(mb_mem_tag_t)(s) == (s))
#define _MB_WRITE_MEM_TAG_SIZE(t, s) (*((mb_mem_tag_t*)((char*)(t) - sizeof(mb_mem_tag_t))) = (mb_mem_tag_t)(s))
#define _MB_READ_MEM_TAG_SIZE(t) (*((mb_mem_tag_t*)((char*)(t) - sizeof(mb_mem_tag_t))))

// String helpers.
static char* my_newstr(size_t s) {
	char* ret = 0;
	size_t rs = s;

#ifdef MB_ENABLE_ALLOC_STAT
	if(!_MB_CHECK_MEM_TAG_SIZE(size_t, s))
		return 0;
	rs += sizeof(mb_mem_tag_t);
#endif /* MB_ENABLE_ALLOC_STAT */
	ret = (char*)my_malloc((unsigned)rs);
	mb_assert(ret);
#ifdef MB_ENABLE_ALLOC_STAT
	ret += sizeof(mb_mem_tag_t);
	_MB_WRITE_MEM_TAG_SIZE(ret, s);
#endif /* MB_ENABLE_ALLOC_STAT */

	return ret;
}

static void my_freestr(char* p) {
	mb_assert(p);

#ifdef MB_ENABLE_ALLOC_STAT
	do {
		size_t os = _MB_READ_MEM_TAG_SIZE(p);
		(void)os;
		p = p - sizeof(mb_mem_tag_t);
	} while(0);
#endif /* MB_ENABLE_ALLOC_STAT */
	my_free(p);
}

static char* my_strdup(const char* p, size_t s) {
#ifdef MB_ENABLE_ALLOC_STAT
	if(!s) {
		s = _MB_READ_MEM_TAG_SIZE(p);
	}

	return mb_memdup(p, (unsigned)s);
#else /* MB_ENABLE_ALLOC_STAT */
	if(s)
		return mb_memdup(p, (unsigned)s);

	return mb_memdup(p, (unsigned)(mb_strlen(p) + 1));
#endif /* MB_ENABLE_ALLOC_STAT */
}

static unsigned my_strlen(const char* s) {
#ifdef MB_ENABLE_ALLOC_STAT
	size_t sz = _MB_READ_MEM_TAG_SIZE(s);
	if(sz > 0)
		--sz; // Minus the trailing \0.

	return sz;
#else /* MB_ENABLE_ALLOC_STAT */
	return strlen(s);
#endif /* MB_ENABLE_ALLOC_STAT */
}

// Test APIs.
static int my_produce(struct mb_interpreter_t* s, void** l) {
	int result = MB_FUNC_OK;
	mb_value_t arg;
	mb_value_t ret;

	mb_assert(s && l);

	mb_make_nil(arg);
	mb_make_nil(ret);

	mb_check(mb_attempt_open_bracket(s, l));
	mb_check(mb_pop_value(s, l, &arg));
	mb_check(mb_attempt_close_bracket(s, l));

	const char txt[] = "Hello\0World\0!";
	const size_t sz = countof(txt);
	char* ptr = my_strdup(txt, sz);
	mb_value_t val;
	mb_make_string(val, ptr);

	const size_t len1 = my_strlen(arg.value.string);
	const size_t len2 = my_strlen(val.value.string);
	char* newstr = my_newstr(len1 + len2 + 1);
	memcpy(newstr, arg.value.string, len1);
	memcpy(newstr + len1, val.value.string, len2);
	newstr[len1 + len2] = '\0';
	for(size_t i = 0; i < len1 + len2; ++i) {
		newstr[i] = (char)toupper(newstr[i]);
	}
	my_freestr(ptr); ptr = 0;

	mb_make_string(ret, newstr);
	mb_check(mb_push_managed_value(s, l, ret, true)); // This string is managed by the core.
                                                          // So no need to free it manually.

	return result;
}

static int my_consume(struct mb_interpreter_t* s, void** l) {
	int result = MB_FUNC_OK;
	mb_value_t arg;

	mb_assert(s && l);

	mb_make_nil(arg);

	mb_check(mb_attempt_open_bracket(s, l));
	mb_check(mb_pop_value(s, l, &arg));
	mb_check(mb_attempt_close_bracket(s, l));

	const size_t len1 = my_strlen(arg.value.string);
	for(size_t i = 0; i < len1; ++i) {
		char ch = arg.value.string[i];
		if(ch == '\0')
			printf("\\0, ");
		else
			printf("%c, ", arg.value.string[i]);
	}

	return result;
}

// Demo code.
int main() {
	mb_set_memory_manager(my_malloc, my_free);
	mb_set_string_measurer(my_strlen);

	struct mb_interpreter_t* bas = NULL;

	mb_init();
	mb_open(&bas);
	mb_reg_fun(bas, my_produce);
	mb_reg_fun(bas, my_consume);
	mb_load_string(bas, "let foo = my_produce(\"Amazing!\")\nmy_consume(foo)", true);
	mb_run(bas, true);
	mb_close(&bas);
	mb_dispose();

	return 0;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants