jinganix / enif_protobuf

A Google Protobuf implementation with enif (Erlang nif)
38 stars 21 forks source link

proto3: field specific custom options #17

Closed balusu-murali closed 2 years ago

balusu-murali commented 3 years ago

Hi, I am using gpb create the message definitions and then loading them using enif_protobuf for the codec. I need to load a field specific option and then update the encoder and decoder for that field.. specifically i want to achieve the following:

in the following definition: i have added a custom option to the integer field indicating binary = true.. this means that the integer value when decoded should be returned as a binary value and during encoding: convert the given binary to integer and then encode it.. for the 2nd integer field without the custom option - the usual logic can run.

syntax = "proto3";
message One {
    int32 id = 1 [binary = true];
        int32 j = 2;
}

how do i achieve this? i am able to get this from the message definitions at gpb.. so i need to be able to store this in the cache and then update the encoder and decoder for this. any pointers would be greatly appreciated.

i am trying to add it as an option to the field similar to how 'packed' is being used.. would that be okay?

jinganix commented 3 years ago

Yes, I think you can refer to 'packed' option.

balusu-murali commented 3 years ago

@jg513 hi, i managed to make the change here: https://github.com/HalloAppInc/enif_protobuf/pull/9/files i mainly want it to be working for 64-bit integers and the option_name is 'ebin' - when set to true - convert binary to integer over the wire.. and after decoding convert it back to a binary. it works when i tested.. i wanted to get your comments if i'm missing something. could you please take a look. would really appreciate any thoughts/suggestions. thanks in advance.

balusu-murali commented 3 years ago

@jg513: can you please take a look and let me know if you see something odd.

jinganix commented 3 years ago

Sorry for late response. I have left a comment, please take a look.

balusu-murali commented 3 years ago

@jg513: thank you for your response. really appreciate it. updated it now - looks a lot cleaner i guess. please take another look. https://github.com/HalloAppInc/enif_protobuf/pull/9/files

balusu-murali commented 3 years ago

i also needed to handle the case where we receive a binary without the field at all.. in these cases: i needed an empty binary. so had to do this fix: https://github.com/HalloAppInc/enif_protobuf/pull/9/commits/1792b7fcd3ae838c0af1b153ca1d713dd6d44aa9

using state->binary_nil was causing a crash.. not sure why. @jg513 could you please take a look at the change again! thank you.

jinganix commented 3 years ago

I view the code, and I think it good. I don't know why it crashed, too. Maybe you can dump it.

balusu-murali commented 3 years ago

@jg513 you mean - debug it further? how do i do it?

jinganix commented 3 years ago

Search core dump.

balusu-murali commented 3 years ago

@jg513 got it, thanks. also - just so you know - i had to switch to custom function to convert char to integer - as atol was causing some issues for some 64 bit integers.

jinganix commented 3 years ago

What is the issue? I found one implementation of atol, I think it is the same as you implemented before. http://www.beedub.com/Sprite093/src/lib/c/stdlib/atol.c