added deep-copy functions tosc_copy_message and tosc_copy_bundle

[neonsoftware]

Added two simple deep copy functions.

As you can see the memory allocation is completely in charge of the caller (as warned within the function comments), which results in the functions being very simple, which I guess might make them quite light to maintain in the future.

Anyways please feel absolutely free to close if you don't find them useful in the API. (I can move this feature in the client)

Please also feel free to return any feedback or demand for any change or modify it.

Cheers again 👍

[mhroth]

Hi @neonsoftware, thanks a lot for the PR! I can certainly see the usecase for a deep copy function. In order to simplify what you've proposed, what do you think of something like:

int tosc_copy_message(tosc_message *d_o, char *d_buffer, int d_len, tosc_message *src_o) {
  if (d_len < src_o->len) return -1;
  memcpy(d_buffer, src_o->buffer, src_o->len);
  tosc_parseMessage(d_o, d_buffer, src_o->len);
  return 0;
}

[neonsoftware]

Hi @mhroth. calling the parser ? Even better !

[neonsoftware]

let me know if the PR is to be re-done or the change will be on your side

[mhroth]

I realise that this PR is dragging on, I'm sorry for that. Could you write a code snippet for how you intend to use this function? This function is now only saving you two lines of code (one to copy the buffer, one to parse the buffer). I'm thinking that the usecase is to copy the OSC buffer away from the networking code and onto some other queue. To me it feels like if that's what you are trying to accomplish, then, it seems to me, you wouldn't need this kind of a deep copy function. You could just call memcpy(d_buffer, src_o->buffer, src_o->len) manually?

I hope that you don't misunderstand my intentions. I really appreciate the PR, and I'd like to balance that with keeping the library, well, tiny ;)

[neonsoftware]

No problem at all ! 👍 Please always take all the time it takes to defend quality. It's a fast world, they say, but it's also wrong and broken. It's ready when it's ready. 😄 Thanks to you for taking the time to maintain.

But here is the current usage context, absolutely, please feel free to see where they are used, here in this repo, for example, search toss_copy in the file.

kian_next_message() looks in a pump to check if a message has arrived. The caller of kian_get_next_message is going to stack the message up in an array to use them after, so it needs to strictly deep-copy.

int kian_get_next_message(tosc_message *dst)
{
	tosc_message *next_msg = NULL;

	if (!kian_bag.is_initialized)
		kian_init();

	next_msg = kian_next_message();
	if (next_msg != NULL) {
		tosc_copy_message(dst, next_msg);
		return 1;
	}
	return 0;
}

if I substitute I would still need also a tosc_parseMessage, please feel me if I'm wrong

int kian_get_next_message(tosc_message *dst)
{
	tosc_message *next_msg = NULL;

	if (!kian_bag.is_initialized)
		kian_init();

	next_msg = kian_next_message();
	if (next_msg != NULL) {
                memcpy(dst->buffer, next_msg-> buffer, dst->len);
                tosc_parseMessage(dst, dst->buffer, dst->len);
		return 1;
	}
	return 0;
}

Two aspects :

• I actually did not, even in my code which must improve, verify the return of the buffer length check(-1). In that case I would have, in the second version, have to also add the length check (if (d_len < dst->len) return -1). But let's keep this apart, and the number of code lines would have been the same.
• Looking at the code, the indirections and access to internal data structure are quite explicit (->buffer, ->len), are white-box, no encapsulation or abstraction as in the rest of the API, the user needs to know about about the exact internals. You would not be able to change the namings (you want to rename len to buflen) in tosc_message without breaking deep copies. Here the motivation for the addition of the deep-copy API.