allocate c strings in metadata functions

[kallewoof]

The current implementation with metadata string handling is complex and error-prone, which has lead to bugs. This code requires the caller to deallocate the returned string, but in return, the interface is a lot simpler.

• I have read the contributing guidelines
• Self-reported review complexity:
  • Low
  • Medium
  • High

Alternative to #10419.

[slaren]

We cannot allocate memory that the user is responsible for freeing, they may not even have access to the same stdlib.

[kallewoof]

We cannot allocate memory that the user is responsible for freeing, they may not even have access to the same stdlib.

I see. So we are

• declaring the function in C land (so no C++ stuff),
• storing as std::string (C++ turf),
• but we copy into a char* for use in C code (as our declaration demands it),
• which we then store as an std::vector (on the other end, once more in C++ land)
• which we then copy over into an std::string
• which we coincidentally do wrong (because we NULL the last byte).

I'm not really surprised that this is the case. This looks awfully convoluted.

Can we maybe use C strings instead of std::strings, at least for the values? These are never deallocated AFAICT, so memory wise this should be equivalent. Probably slightly lighter even.

    std::unordered_map<std::string, char*> gguf_kv;

I opened this as #10430 and am closing this one.