llohse / libnpy

C++ library for reading and writing of numpy's .npy files
MIT License
361 stars 72 forks source link

Work around Android toolchains issues #3

Closed AnthonyBarbier closed 6 years ago

AnthonyBarbier commented 7 years ago

Android toolchains are missing std::to_string, this patch works around this issue

llohse commented 7 years ago

Thank you. I will look into this tomorrow.

llohse commented 7 years ago

Thanks again for bringing this up and proving a patch. I rewrote it a little bit to simplify the overall code. Should be fixed with d92bcbcb7c247e0b0ccaeb653298caf6b58f6fc3.

Can you please confirm?

AnthonyBarbier commented 7 years ago

No, it doesn't work, I get "invalid header" when trying to load files with your latest header, so there must be something missing.

llohse commented 7 years ago

That is weird because "invalid header" is thrown prior to the lines affected by the change, when the npy file misses the '\n' trailing the header. Are you certain that it worked before?

AnthonyBarbier commented 6 years ago

It's because you're missing header_length I think, the following changes seem to be enough to solve the issue

diff --git a/include/libnpy/npy.hpp b/include/libnpy/npy.hpp
index d072b77..e0d514a 100644
--- a/include/libnpy/npy.hpp
+++ b/include/libnpy/npy.hpp
@@ -352,7 +352,7 @@ inline std::string read_header_1_0(std::istream& istream) {

     char *buf = new char[header_length];
     istream.read(buf, header_length);
-    std::string header (buf);
+    std::string header (buf,header_length);
     delete[] buf;

     return header;
@@ -371,7 +371,7 @@ inline std::string read_header_2_0(std::istream& istream) {

     char *buf = new char[header_length];
     istream.read(buf, header_length);
-    std::string header (buf);
+    std::string header (buf,header_length);
     delete[] buf;

     return header;
llohse commented 6 years ago

Thanks again! Try the current commit. Note that it contains some other improvements.

AnthonyBarbier commented 6 years ago

Still doesn't work on Android:

27/09/2017 10:28:41:ERROR:scons_builder2:ThreadPool:include/libnpy/npy.hpp:313:57: error: 'htole16' was not declared in this scope
27/09/2017 10:28:41:ERROR:scons_builder2:ThreadPool:       uint16_t header_len_le16 = htole16(header.length());
27/09/2017 10:28:41:ERROR:scons_builder2:ThreadPool:                                                         ^
27/09/2017 10:28:41:ERROR:scons_builder2:ThreadPool:include/libnpy/npy.hpp:316:57: error: 'htole32' was not declared in this scope
27/09/2017 10:28:41:ERROR:scons_builder2:ThreadPool:       uint32_t header_len_le32 = htole32(header.length());
27/09/2017 10:28:41:ERROR:scons_builder2:ThreadPool:                                                         ^
27/09/2017 10:28:41:ERROR:scons_builder2:ThreadPool:include/libnpy/npy.hpp: In function 'std::string npy::read_header_1_0(std::istream&)':
27/09/2017 10:28:41:ERROR:scons_builder2:ThreadPool:include/libnpy/npy.hpp:328:55: error: 'le16toh' was not declared in this scope
27/09/2017 10:28:41:ERROR:scons_builder2:ThreadPool:     uint16_t header_length = le16toh(header_length_raw);
27/09/2017 10:28:41:ERROR:scons_builder2:ThreadPool:                                                       ^
27/09/2017 10:28:41:ERROR:scons_builder2:ThreadPool:include/libnpy/npy.hpp: In function 'std::string npy::read_header_2_0(std::istream&)':
27/09/2017 10:28:41:ERROR:scons_builder2:ThreadPool:include/libnpy/npy.hpp:347:55: error: 'le32toh' was not declared in this scope
27/09/2017 10:28:41:ERROR:scons_builder2:ThreadPool:     uint32_t header_length = le32toh(header_length_raw);
llohse commented 6 years ago

... obviously. Because I removed endian.h since it was not required with my version of gcc. The latest commit includes portable byte order conversion. Please check, if it works now.

If you tell me what compiler you are using I will test it myself next time I change something.

AnthonyBarbier commented 6 years ago

Seems to be better but there are some warnings which will prevent the header to compile with -Werror enabled:

include/libnpy/npy.hpp|358 col 31| error: array index 2 is past the end of the array (which contains 2 elements) [-Werror,-Warray-bounds]
||                            | (header_len_le32[2] << 16) | (header_len_le32[3] <<  24);
||                               ^               ~
include/libnpy/npy.hpp|354 col 5| note: array 'header_len_le32' declared here
||     char header_len_le32[2];
||     ^
include/libnpy/npy.hpp|358 col 60| error: array index 3 is past the end of the array (which contains 2 elements) [-Werror,-Warray-bounds]
||                            | (header_len_le32[2] << 16) | (header_len_le32[3] <<  24);
||                                                            ^               ~
include/libnpy/npy.hpp|354 col 5| note: array 'header_len_le32' declared here
||     char header_len_le32[2];

I'm using the NDK r14:

AnthonyBarbier commented 6 years ago

Could you please just change the following ?

diff --git a/npy.hpp b/npy.hpp
index 614a050..9b6f7fb 100644
--- a/npy.hpp
+++ b/npy.hpp
@@ -351,7 +351,7 @@ inline std::string read_header_1_0(std::istream& istream) {

 inline std::string read_header_2_0(std::istream& istream) {
     // read header length and convert from little endian
-    char header_len_le32[2];
+    char header_len_le32[4];
     istream.read(header_len_le32, 4);

     uint32_t header_length = (header_len_le32[0] <<  0) | (header_len_le32[1] <<  8)
llohse commented 6 years ago

Thanks! Fixed. Hopefully...

AnthonyBarbier commented 6 years ago

All good ! Thanks!