[Bro-Dev] [JIRA] (BIT-1270) topic/gilbert/plugin-api-tweak

[Gilbert Clark]

Yeah, I think we're in agreement that pass-by-value would be a better 
method here.

To be honest, I'm not sure if expecting a user to return a 
std::pair<bool, Val*> from a plugin function hook is more or less 
obvious than an explicit wrapper.  One thing that makes this a little 
less obvious to me is that the usage of this construct won't be limited 
to bro folks reviewing this code: plugin authors will need to craft one 
of these (either ValWrapper or std::pair) and return them from the 
hook.  std::pair might actually be a little easier, since that might 
make it more obvious what's happening there (since std::pair is vanilla 
C++) ... but it also might not.

How about I give std::pair try and we can move back to an explicit 
wrapper if it makes the code too difficult to work with?  It should be a 
pretty small change either way.

Also, Jon: primary use of this construct (that I can think of) will be 
in HandlePluginResult and BroFunc::Call in Func.cc (since those deal 
with the HookFunctionCall interface) if you'd like to take a look.  
There's also https://github.com/cubic1271/bro-plugin-instrumentation to 
offer an example of a plugin that uses the updated interface - once I 
make this change in core, I'll update that plugin to reflect the new usage.

On a related note, C++11 would be cool for a lot of reasons (e.g. 
std::atomic), but that might be another ticket / a longer discussion :)

-Gilbert

On 10/17/2014 11:53 AM, Jon Siwek (JIRA) wrote:
>      [ https://bro-tracker.atlassian.net/browse/BIT-1270?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18410#comment-18410 ]
>
> Jon Siwek commented on BIT-1270:
> --------------------------------
>
> Whether to pass by value and whether to use ValWrapper vs. std::pair seem like separate issues/suggestions?
>
> I'd also make the suggestion to pass by value.  Whether that value is ValWrapper or std::pair I don't have a good feel for without reviewing the code more.  Maybe std::pair if it's used infrequently else ValWrapper just to be able to chose the names of the fields (reading/writing code that uses "x.first" and "x.second" can get confusing for me personally).
>
> (And std::tuple is C++11, right?  Did we decide that's generally ok to use in Bro now?)
>
>> topic/gilbert/plugin-api-tweak
>> ------------------------------
>>
>>                  Key: BIT-1270
>>                  URL: https://bro-tracker.atlassian.net/browse/BIT-1270
>>              Project: Bro Issue Tracker
>>           Issue Type: Improvement
>>           Components: Bro
>>             Reporter: gclark
>>             Assignee: gclark
>>
>> This branch makes a few changes to the API:
>> * Wraps values in a simple class (ValWrapper) that include an explicit processed / not processed flag (to avoid confusion with delayed / opaque invocations).
>> * Adds a Frame argument to HookCallFunction
>> * Adds support for Frame argument types to HookArgument
>> * Adds support for ValWrapper argument types to HookArgument
>> * Tweaks the plugin.hooks tests a bit to include new output (from additional argument)
>> * Tweaks the plugin.api-version-mismatch to remove explicit home directory path via simple regex
>
>
> --
> This message was sent by Atlassian JIRA
> (v6.4-OD-07-004#64005)
> _______________________________________________
> bro-dev mailing list
> bro-dev at bro.org
> http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev