radare / spp

simple preprocessor
MIT License
60 stars 11 forks source link

Add Meson build #20

Closed ret2libc closed 6 years ago

radare commented 6 years ago

Yep i know, this was kind of a hack to get this into r2 without too much hassle or dupplicated code. maybe the better solution would be to rename this rapi code (which is probably outdated compared to the current state of the implementation). all the code using r* apis inside spp is not public, so we dont need to expose or make this reusable from outside imho, so adding this option just complicates the build system for just 3 functions that process strings. i would go for making them more specific to spp. what do you think?

On 3 Aug 2018, at 12:16, Riccardo Schirone notifications@github.com wrote:

@ret2libc commented on this pull request.

In meson.build https://github.com/radare/spp/pull/20#discussion_r207502348:

@@ -0,0 +1,40 @@ +project('spp', 'c') + +platform_inc = ['.'] +c_args = [] + +spp_files = [

  • 'spp.c' +]
  • +if get_option('use_r_util') and get_option('r_util_includes') != '' r_api is not required. Unfortunately SPP is a bit problematic. If you compile it standalone, r_api.c is used to define rstrbuf, rsys and other used functions.

But when used inside radare2, rstrbuf and r_sys functions are provided by r_util.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/radare/spp/pull/20#discussion_r207502348, or mute the thread https://github.com/notifications/unsubscribe-auth/AA3-lnAJhQW5rbZwryGI80VFd7A6PrdMks5uNCMBgaJpZM4VsDGc.

ret2libc commented 6 years ago

I thought a bit about this problem, but I couldn't really come up with a good solution. The ideal solution IMHO would be to have a rutil library which can be self-compiled without spp/sdb/etc. and that that can be used by spp as well as other r* libraries.

I'm not sure we are talking about the same thing though. Those options I added are not to make r* functions in spp accessible from outside. They are a way for r2 to provide its own versions of r* functions, instead of using the (old) embedded one in spp.

Are you suggesting that we just throw away the r_* functions in spp and we write simpler code there, that doesn't require them?

radare commented 6 years ago

spp only uses those functions to optimize the “strcat” with a stringbuffer style api. i didnt wanted to redo the same thing i did in r2, so i opted for copypasting that into r_api.c

imho i would go for making those functions spp-specific, so, not public, they are not that big, and we dont need to expose them, its just that we want a way to concat strings in an optimal way, to finally get a char*

On 3 Aug 2018, at 16:15, Riccardo Schirone notifications@github.com wrote:

I thought a bit about this problem, but I couldn't really come up with a good solution. The ideal solution IMHO would be to have a rutil library which can be self-compiled without spp/sdb/etc. and that that can be used by spp as well as other r* libraries.

I'm not sure we are talking about the same thing though. Those options I added are not to make r* functions in spp accessible from outside. They are a way for r2 to provide its own versions of r* functions, instead of using the (old) embedded one in spp.

Are you suggesting that we just throw away the r_* functions in spp and we write simpler code there, that doesn't require them?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/radare/spp/pull/20#issuecomment-410263321, or mute the thread https://github.com/notifications/unsubscribe-auth/AA3-llKRMrnFgA_xXrgn4fL3fcO1VUchks5uNFsNgaJpZM4VsDGc.

ret2libc commented 6 years ago

There are also r_sys functions and others, but I’ll look better at the code and see if we can remove them

ret2libc commented 6 years ago

So what do you think of this? I just always use the local definitions of sstrbuf and ssys. This, however, means that when spp will be linked with radare2's code, there would be some "duplicated functions" that do the same stuff.

radare commented 6 years ago

Thats exactly what i said. Just rename them and make them static and non gloably visible. And probably make them simpler and more optimized for the only usecase we have

On 3 Aug 2018, at 19:39, Riccardo Schirone notifications@github.com wrote:

So what do you think of this? I just always use the local definitions of sstrbuf and ssys. This, however, means that when spp will be linked with radare2's code, there would be some "duplicated functions" that do the same stuff.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.