[Oisf-devel] (no subject)

[Pierre Chifflier]

> Hi Pierre, sorry for the late reply. I have some questions & comments.
> 
Hi Victor,
Here's an updated version of the second part of the patch.

> - calls to the logging api should not include a newline
Fixed

> - can you move the IPH_IS_VALID macro to decode.h?
Done

> - can you comment the functions in doxygen compatible format?
I've added comments for all functions

> - if in EventToImpact idmef_assessment_new_impact fails, will there be
> memory lost? Same for EventToSourceTarget and other functions.
Nope, objects are referenced in the parent struct, so freeing
the entire idmef_alert_t object is enough.

> - can you add the SCEnter and SCReturn* macro's for debugging?
Done

> - instead of "if (p->ip4h)" you can use "if (PKT_IS_IPV4(p))" same for
> TCP and UDP
Done

> - AddByteData adds p->pkt which contains the full packet including
> headers. p->payload contains the payload. Is the full pkt what you want?
Good catch, thanks

> - AlertPreludeThreadInit leaks mem on error
Fixed

> - AlertPreludeThreadDeinit checks for aun == NULL, if it is we jump to
> error. There we check again... seems redundant.
Fixed

> - AlertPreludeInitCtx uses malloc without checking it's retval
Fixed (though I'm pretty sure that if you can't allocate 4 bytes you'll
have other problems ;)

> - I'm not really happy about the use of the LogFileCtx, and I guess
> neither are you... I guess we need to generalize that a bit.
That's the only point I haven't changed, since I'm not sure of the
solution (as discussed). IMHO, LogFileCtx should be generalized, but
that requires more changes.
Would you like me to do the change, or is it planned in master ?

> - the prelude-profile option is not present in the example config
Added

Cheers,
Pierre