Hi,
Trying to implement a method that returns a string, I wonder if there is any "standard" for skia/skiasharp in how to do it.
I looked at what is already implemented, but the only methods that return strings seem to be:
const char* sk_imagedecoder_get_format_name_from_format (sk_imagedecoder_format_t cformat);
const char* sk_imagedecoder_get_format_name_from_decoder (sk_imagedecoder_t* cdecoder);
And they don't seem to work fine. They aren't either in the "official" skia distribution yet ( https://github.com/google/skia/tree/master/include/c )
The main issue is with memory management. For example, sk_imagedecoder_get_format_name_from_decoder returns a char* which is generated here:
const char* SkImageDecoder::GetFormatName(Format format) {
switch (format) {
case kUnknown_Format:
return "Unknown Format";
case kBMP_Format:
return "BMP";
case kGIF_Format:
return "GIF";
...
So in this case we are returning constants, and no memory is allocated in the C side. But on the other hand, they shouldn't be deallocated by the C# bindings either.
The current binding declaration is:
[DllImport(SKIA, CallingConvention = CallingConvention.Cdecl)]
public extern static string sk_imagedecoder_get_format_name_from_decoder(sk_imagedecoder_t cdecoder);
And accodring to this:
http://stackoverflow.com/questions/370079/pinvoke-for-c-function-that-returns-char
This means that the CLR assumes it needs to free the native memory, and that the memory was allocated with CoTaskMemAlloc. None of this is true, in fact, I don't even know which would be the assumptions in OSX or somewhere else where CoTaskMemAlloc don't exist. As the CLR will try to free the unmanaged memory, but that memory was a constant (and not allocated with CoTaskMemAlloc), this should crash.
And indeed, if I try:
var ms = new FileStream("r:\\test.png", FileMode.Open);
var mst = new SKManagedStream(ms);
SKImageDecoder k = new SKImageDecoder(mst);
string s = k.FormatName;
The app crashes.
So, given that there doesn't seem to be any example of returning strings in the original bindings, and the new ones don's seem to work correctly, I think it would be important to determine an standard on how to return strings from the C api. I could add my own function (GetFontName) with my own specification, but then other functions returning strings would be either inconsistent or forced to follow what I did. Functions returning strings shouldn't be very common in a graphics API, but still I think it is a good thing to have an standard.
some ideas:
- We could use something similar to MultiByteToWideChar in Windows ( https://msdn.microsoft.com/en-us/library/windows/desktop/dd319072(v=vs.85).aspx )
In this case, you would define a function that returns the lenght of the needed buffer and copies characters to the buffer if it is not null. So you call the function 2 times:
//pseudocode
GetFontName(&len, NULL);
malloc(buffer, len);
GetFontName(len, buffer);
In the first call you get the size of the needed buffer, then you allocate the buffer and call it again to get the characters copied into the buffer.
Probably the drawback of this is that it is prone to errors, and confessions between char counts and byte counts.
As it says on the top of the linked page fro MultiByteToWideChar:
Caution Using the MultiByteToWideChar function incorrectly can compromise the security of your application. Calling this function can easily cause a buffer overrun because the size of the input buffer indicated by lpMultiByteStr equals the number of bytes in the string, while the size of the output buffer indicated by lpWideCharStr equals the number of characters. To avoid a buffer overrun, your application must specify a buffer size appropriate for the data type the buffer receives. For more information, see Security
The advantage is that memory is allocated and freed in the client.
2)We could have the memory allocated in the C api, and require the calling application to call some
SKIA_FREE(buffer)
when it is done using the buffer.
It makes it more difficult to have buffer overflows, but it makes it simpler to forget to call SKIA_FREE and call the standard Free in the client side and cause a crash.
3)We could return SKString objects, which would have to be freed like any other Skia object. But we still would have to determine if the SKString owns the memory or not when you get the string from the Skia object. Given that I don't expect many methods returning strings, I think creating a SKString wrapper is probably overkill.
This is what I can think of. Any other ideas?