{"action":"created","issue":{"url":"https://api.github.com/repos/gonum/hdf5/issues/20","repository_url":"https://api.github.com/repos/gonum/hdf5","labels_url":"https://api.github.com/repos/gonum/hdf5/issues/20/labels{/name}","comments_url":"https://api.github.com/repos/gonum/hdf5/issues/20/comments","events_url":"https://api.github.com/repos/gonum/hdf5/issues/20/events","html_url":"https://github.com/gonum/hdf5/pull/20","id":325682968,"number":20,"title":"[Experimental] Marshal C Data to []byte","user":{"login":"TacoVox","id":3264648,"avatar_url":"https://avatars2.githubusercontent.com/u/3264648?v=4","gravatar_id":"","url":"https://api.github.com/users/TacoVox","html_url":"https://github.com/TacoVox","followers_url":"https://api.github.com/users/TacoVox/followers","following_url":"https://api.github.com/users/TacoVox/following{/other_user}","gists_url":"https://api.github.com/users/TacoVox/gists{/gist_id}","starred_url":"https://api.github.com/users/TacoVox/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/TacoVox/subscriptions","organizations_url":"https://api.github.com/users/TacoVox/orgs","repos_url":"https://api.github.com/users/TacoVox/repos","events_url":"https://api.github.com/users/TacoVox/events{/privacy}","received_events_url":"https://api.github.com/users/TacoVox/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":8,"created_at":"2018-05-23T12:29:01Z","updated_at":"2018-05-24T08:40:59Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/gonum/hdf5/pulls/20","html_url":"https://github.com/gonum/hdf5/pull/20","diff_url":"https://github.com/gonum/hdf5/pull/20.diff","patch_url":"https://github.com/gonum/hdf5/pull/20.patch"},"body":"First and foremost: this is an experimental P/R that works. But the main purpose is for me to get your feedback and maybe we can work something out that is nice and could replace at least the suboptimal `C.realloc` calls.\r\n\r\nAs @sbinet pointed out in https://github.com/gonum/hdf5/pull/17, it could be nice to marshal data into a `[]byte`. I coulndn't wait, started coding on another branch, and this is the result:\r\n- Replaced the nasty `C.realloc` calls with a simple `make` and `copy` to increase the `[]byte` size.\r\n- Got rid of the `C.free` that was necessary as a result of the memory allocation.\r\n- Experimented a bit and I think I found an acceptable way to find the \"native\" endianess of the machine. It seems to work as expected.\r\n- Replaced most of the C types with Go-native ones.\r\n\r\nWhat I was not able to achieve is to get rid of the usage of `unsafe` related things. I hope we can find a solution for that. If I am not mistaken, it is obviously a problem to marshal `C.float` and `C.double` into a `uint32` or `uint64` without using `unsafe.Pointer` and a cast. But while experimenting, I found the same for the `unsafe.Pointer` address to the `C.String`s, and the signed integers. A potential solution would have been to extend the `ad.buffer` by 8 bytes before the case statement and then just use `unsafe.Pointer` in those mentioned cases and for the unsigned integers e.g. a `nativeEndian.PutUint16` instead. But for consistency, I liked this solution more.\r\n\r\nFurthermore, the strings are as they are. I think, limited by the HDF5 library itself, there is no other way for us to move away from that.\r\n\r\nWith that: I would be very glad about any input. There is no rush in getting this PR in, I just wanted to exchange thoughts with you guys on that.\r\n\r\nIf and only if we will get this in, I will change the commit message ;)"},"comment":{"url":"https://api.github.com/repos/gonum/hdf5/issues/comments/391636349","html_url":"https://github.com/gonum/hdf5/pull/20#issuecomment-391636349","issue_url":"https://api.github.com/repos/gonum/hdf5/issues/20","id":391636349,"user":{"login":"TacoVox","id":3264648,"avatar_url":"https://avatars2.githubusercontent.com/u/3264648?v=4","gravatar_id":"","url":"https://api.github.com/users/TacoVox","html_url":"https://github.com/TacoVox","followers_url":"https://api.github.com/users/TacoVox/followers","following_url":"https://api.github.com/users/TacoVox/following{/other_user}","gists_url":"https://api.github.com/users/TacoVox/gists{/gist_id}","starred_url":"https://api.github.com/users/TacoVox/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/TacoVox/subscriptions","organizations_url":"https://api.github.com/users/TacoVox/orgs","repos_url":"https://api.github.com/users/TacoVox/repos","events_url":"https://api.github.com/users/TacoVox/events{/privacy}","received_events_url":"https://api.github.com/users/TacoVox/received_events","type":"User","site_admin":false},"created_at":"2018-05-24T08:40:59Z","updated_at":"2018-05-24T08:40:59Z","author_association":"CONTRIBUTOR","body":"As you can see, I moved to a method of that `encoder` type. That makes sense to me and looks IMHO  good.\r\nThe only thing that I am not really fond of is the support of a `CMarhsaller` interface. Why would we like to support that? Is that not causing more issues that it could solve for the end-user?"}}
{"actor":{"display_login":"TacoVox"}}