bytes_to_str with string as encoding.

[yav]

It would appear that the Kaitait compiler generates calls to bytes_to_str, where the encoding parameter is std::string instead of char *, so I had to add a wrapper function to make things build:

diff --git a/kaitai/kaitaistream.cpp b/kaitai/kaitaistream.cpp                   
index f3d95eb..9e01bcd 100644                                                       
--- a/kaitai/kaitaistream.cpp                                                       
+++ b/kaitai/kaitaistream.cpp                                                    
@@ -655,6 +655,10 @@ uint8_t kaitai::kstream::byte_array_max(const std::string val) {
 #include <cerrno>                                                               
 #include <stdexcept>                                                            
.                                                                                
+std::string kaitai::kstream::bytes_to_str(const std::string src, const std::string enc) {
+  return kaitai::kstream::bytes_to_str(src, enc.c_str());                       
+}                                                                               
+                                                                                
 std::string kaitai::kstream::bytes_to_str(const std::string src, const char *src_enc) {
     iconv_t cd = iconv_open(KS_STR_DEFAULT_ENCODING, src_enc);                  
.                                                                                
diff --git a/kaitai/kaitaistream.h b/kaitai/kaitaistream.h                       
index 3523ed1..6f8729d 100644                                                    
--- a/kaitai/kaitaistream.h                                                      
+++ b/kaitai/kaitaistream.h                                                      
@@ -167,6 +167,7 @@ public:                                                      
.                                                                                
     static std::string bytes_strip_right(std::string src, char pad_byte);          
     static std::string bytes_terminate(std::string src, char term, bool include);
+    static std::string bytes_to_str(const std::string src, const std::string enc);
     static std::string bytes_to_str(const std::string src, const char *src_enc);
.                                                                                
     //@}            

[generalmimon]

The general answer is that this is caused by a mismatch between the KS compiler version and the KS C++ runtime library version. If you use a released "stable" compiler from https://github.com/kaitai-io/kaitai_struct_compiler/releases/tag/0.10, the generated C++ code is meant to operate with the 0.10 tag of the C++ runtime.

Although the generated C++ code contains a compile-time check for the mismatch between the version of the compiler (KSC) used to generate it and the C++ runtime library (kaitai-io/kaitai_struct#110), currently it's only designed for the case when the runtime library is older than it should be, not when the runtime library is newer than the generated code. It seems that the original idea of the linked issue was to diagnose the opposite direction as well, but it hasn't been implemented yet - see kaitai-io/kaitai_struct#110 (comment):

Right now it's possible to generate a target file, say with compiler = 0.6, and use runtime = 0.5, or vice versa. Both will potentially break on some API incompatibility.

[generalmimon]

To be honest (cc @GreyCat), I personally don't see the benefits of changing the type of parameter src_enc from const std::string to const char *. According to #61 (comment), it's supposed to help with a future optimization of resolving encoding names at compile time:

• This conversion (string->integer) is costly (and mostly unnecessary to do in runtime), so in future I will offer a way to do convert encoding string in compile-time.
• To be able to do that, I've started switching from std::string src_enc to const char* src_enc, which paves the way to optimize it further.

But I don't know C++ well enough to appreciate that or to be able to see how it helps specifically - is std::string not working in constexpr context, but const char * is, or what exactly is the point?

One other "advantage" I can think of is that using const char * might indicate to the caller that a plain string literal is expected (as explained in kaitai-io/kaitai_struct#1051), rather than a dynamic string.

Nevertheless, I think that breaking backwards compatibility caused by removing the bytes_to_str(const std::string, const std::string) method and only providing bytes_to_str(const std::string, const char *) is probably unnecessary, given that C++ supports method overloading.

[mgrosvenor]

I’ve just been bitten by the same issue. There’s a few things here to consider:

• There are no offical releases, or change log notes. So I have to assume that the “tags” are releases. Adding some offical GitHub releases with notes would make it clearer what’s happening.
• There’s a breaking change on a point release. Generally it’s reasonable to expect a point release is for bug fixes, but adds no breaking changes. ie 0.10.1 should reasonably be expected to be backwards compatible with 0.10.0.
• There’s also no 0.10.1 binary release of Kaitai compiler so there’s no way to run the 0.10.1 code (as far as I can tell)
• There’s no checking at build time that the point release version is the same as the generated code version. Some static checks would alleviate a lot pain here.

[generalmimon]

@mgrosvenor:

• There’s a breaking change on a point release. Generally it’s reasonable to expect a point release is for bug fixes, but adds no breaking changes. ie 0.10.1 should reasonably be expected to be backwards compatible with 0.10.0.
• There’s also no 0.10.1 binary release of Kaitai compiler so there’s no way to run the 0.10.1 code (as far as I can tell)

100%. I also don't understand why @GreyCat made a 0.10.1 version with a breaking change compared to 0.10 (https://github.com/kaitai-io/kaitai_struct_cpp_stl_runtime/compare/0.10..0.10.1#diff-b9569f864c19a6430234c5a161d3ab9e4f921daaae356d4a97dc1e458eef16f6) - I think it was a mistake. Now, it's indeed unclear with which version of KSC the 0.10.1 runtime library should be used.

• There’s no checking at build time that the point release version is the same as the generated code version. Some static checks would alleviate a lot pain here.

Yeah, as I already mentioned above - #67 (comment), the existing version check only works in one direction, and in this case the problem is in the other direction, which is unchecked.