erpc icon indicating copy to clipboard operation
erpc copied to clipboard

option to redefine 'string' or create a 'ustring'

Open pgu-swir opened this issue 5 years ago • 10 comments

One last question (and feel free to ignore it)

The large library I'm making available as a RPC has a lot of protoypes with parameters of type "jerry_char_t" which is typed as an "uint8_t". There are null-terminated string in reality, so the proper (and only) way to use them in IDL is use a "string". The problem of course is then the shim prototypes do not match the "real" prototypes in headers, and I'd really like to keep the real headers and not have erpcgen generate new ones, it helps to catch errors.

For now, I have to compile the library with jerry_char_t defined as an uint8_t and when I use the headers with RPC, I redefine it as a char. This is not very pretty, but not dangerous.

I was wondering is the IDL could be extended to allow string to be re-defined or we could also have a ustring, knowing that generated code would always cast ustring variables to char for calling things like strlen, but that would be a generic rule This signed/unsigned char has always been a C nightmare.

Another option would be to use 'binary' in IDL and let @length() have a function as a parameter

my_func(uint32 p1, string p2 @length(strlen(p2))) -> uint32

Or alternatively, the IDL could allow @length() to receive a binary as a parameter and would then itself call strlen() in the generated code (which is exactly similar to what string does

I'm sorry I cannot propose a patch, but that part of eRPC is black magic to me

pgu-swir avatar Sep 16 '20 06:09 pgu-swir

Hi @pgu-swir , currently we were not be aware of define string as another type than char. @MichalPrincNXP can decide if we would like to to support this use case.

Just question what would happen if we use @external type jerry_char_t = string. Then function declaration should match but not sure with definition.

Hadatko avatar Sep 16 '20 12:09 Hadatko

Code snippet

program a

@external type a=string

interface B{
   oneway f(a c)
}

Hadatko avatar Sep 16 '20 12:09 Hadatko

The problem I had is that jerry_char_t is a "uint8_t" not " a "uint8_t*" so semantically different and I tried that initially but had issues. But its's true that I was less familiar with IDL syntax and the options to use or not the existing headers, so I'll give it a shot again, thanks for reminding me that!

pgu-swir avatar Sep 16 '20 16:09 pgu-swir

So I re-checked and there is no way around, unfortunately.

  • in the headers that come with the library, "jerry_char_t" is defined as an uint8_t and all the function prototypes that use strings expect a "jeryy_char_t*"
  • if I define "jerry_char_t" as a "string" in my IDL, then shim won't compile of course because their prototypes will use "jerry_char_t" and that's not a pointer, the functions expects a "jerry_char_t*"
  • if I created another type, named "erpc_string_t" as IDL "string" and I include a header that defines is a "jerry_char_t*", it still won't work because jerry_char_t is a uint8_t so the shim won't compile on cast error this time

pgu-swir avatar Sep 17 '20 00:09 pgu-swir

hi @pgu-swir , what about have the idl as i proposed but external variable 'a' wouldbe my_jerry_def instead of jerry_char_t. And outside of IDL you would define typedef my_jerry_def *jerry_char_t;

Hadatko avatar Sep 17 '20 01:09 Hadatko

hi @Hadatko, yes, this is what I tried to do but compilation of shims fails because "jerry_char_t" is an uint8_t, so "jerry_def" is an uint8_t*, and in the shim, there are calls to strlen (e.g.) where gcc whines because "jerry_def" is an unsigned char and is not cast to char (-fpermissive)

And if I compile with -fpermissive, because IDL does not add "const" in front of "jerry_def", then when compiling the client shim, c++ name mangling thinks that the function is different than its prototype (it's an external header that I can't modify), so it mangles the name and generate another enrty and end up with a linker error

pgu-swir avatar Sep 17 '20 02:09 pgu-swir

Clear now. So maybe we need only change read write string functions from **char to **void?

Hadatko avatar Sep 17 '20 05:09 Hadatko

Ideally, it might be 2 things

1- possibility to define an alias of a string as a pointer, i.e. something like

type jerry_char_t @ptr string 
or 
type jerry_char_t pstring

so that the generator replaces string by a "const jerry_char_t*"

2- in the generated code, always cast variables that are aliases to pstring in (void*)

I'm sorry I can't propose a PR, but I can try if you give me a quick hint where to start in erpcgen

pgu-swir avatar Sep 17 '20 05:09 pgu-swir

Hi @MichalPrincNXP , you have basically 3 options here how to solve the issue. Can you writte if you want solve this issue and how? Then @pgu-swir can do PR for you. My suggestion if it will work is change likely on two places not generated by erpcgen. @pgu-swir changes are more complex and they need change also in generated code likely.

Hadatko avatar Sep 17 '20 06:09 Hadatko

Thanks so much for looking at this. [edit]: in fact, looking a bit harder I found the template where to cast the kString element, so 2- is solved and if I can find a solution for 1- then I might be able to propose a patch

pgu-swir avatar Sep 17 '20 06:09 pgu-swir