extend gohermes #116
extend gohermes #116
Conversation
Lagovas
commented
Dec 25, 2017
Lagovas
commented
- added go interfaces for credential/keystore/credential store that should be used by golang implementations
- added midhermes wrapper that uses directly golang implementations of interfaces
- move rpc midhermes wrapper to rpc subpackage (because if two wrappers exists in the same folder then golang compile them as one compilation unit and wrappers for local midhermes override units from C midhermes that expects by rpc wrapper)
add interfaces for go implementation of stores go wrappers of go implementations that can be called by c mid_hermes
| @@ -173,3 +173,4 @@ $BIN/hermes_client -command=delete_block -id=$USER_ID -private_key=$PRIVATE_KEY | |||
| if [ ! $? -eq $SUCCESS ]; then | |||
| exit 1 | |||
| fi | |||
| echo "finished successfully" | |||
| return nil | ||
| // TODO: use multierror like https://github.com/hashicorp/go-multierror | ||
| //hermes := mh.mid_hermes | ||
| //C.mid_hermes_destroy(&(hermes)) |
There was a problem hiding this comment.
shouldn't we destroy mh.mid_hermes here?
There was a problem hiding this comment.
mh.wrapper should call mid_hermes_destroy in .Close() because only he know how he initiated it
And one more problem that mid_hermes_destroy call *_destroy for 'stores' that assigned to mh.mid_hermes pointer which will be freed. And if wrapper.Close() will call mid_hermes_destroy too with own pointer of mid_hermes then will be dereferenced null pointer.
for example:
mh.mid_hermes object has pointer with address 0x222 and points to midhermes struct at address 0xAAAA
wrapper has pointer with own address 0x333 and points to the same struct at address 0xAAA
we call mid_hermes_destroy and pass copy of pointer to 0x222. Then it's dereferenced and freed pointer to where it points (0xAAAA), and set 0x222 = NULL
then wrapper call mid_hermes_destroy and pass copy of pointer to 0x333, dereferenced and try to free 0xAAAA and took error
yeah, we can operate with **mid_hermes but more important that only wrapper know how it should be freed
| var block, meta *C.uint8_t | ||
| var block_length, meta_length C.size_t | ||
| if 0 != C.mid_hermes_read_block( | ||
| if status := C.mid_hermes_read_block( |
There was a problem hiding this comment.
is it okay in Go to mix assignment and if statement? as for me, it's easier to read them separately. it's just imho, not critical
There was a problem hiding this comment.
it's ok when this variable doesn't used anywhere more and accessible only in if scope... but yeah, maybe it's better to split...
There was a problem hiding this comment.
maybe during next refactoring :)
|
|
||
| // hermes_credential_store_destroy implement hermes/mid_hermes_ll_interfaces/data_store.h interface | ||
| hermes_status_t hermes_data_store_destroy(hermes_data_store_t **data_store){ | ||
| return HM_SUCCESS; |
There was a problem hiding this comment.
yes :) because it's empty struct that allocated as field (not as pointer to struct in heap) of go struct that will be freed by go gc
| } | ||
| err = gohermes.SetOutBufferPointerFromByteSlice(ownerId, ownerIdPtr, ownerIdLenPtr) | ||
| if err != nil { | ||
| C.free(tokenPtr) |
There was a problem hiding this comment.
shall we free smth else here?
