[Bro-Dev] [JIRA] (BIT-1050) Merge request for DHCP analyzer

Vlad Grigorescu (JIRA) jira at bro-tracker.atlassian.net
Sun Aug 4 06:39:06 PDT 2013


    [ https://bro-tracker.atlassian.net/browse/BIT-1050?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13424#comment-13424 ] 

Vlad Grigorescu commented on BIT-1050:
--------------------------------------

Thanks, Robin. Sorry it didn't merge cleanly.

{quote}
I'm not sure I like the structure with the two policy scripts. As we don't have anything else extracting MAC addresses currently, would it make sense to just move them into one? If not, I'd at least move the one out of misc, and also change the interface so that (1) users don't manipulate the table directly (but call functions instead), and (2) the logging stays internal to the script (the same functions could take care of that). But I'll let Seth take a look at this (or has he already?), assigning ticket to him.
{quote}

Seth and I have talked about it a bit. My original intention was to also add a script for the ARP analyzer. However, in thinking about it some more, that means that if a host is seen by ARP before DHCP, the DHCP hostname won't be logged. Maybe known_devices logs unique (Analyzer, MAC) pairs instead of unique MACs?

{quote}
format_mac(): The comment says "Supports both EUI-48 and EUI-64. If it's neither, returns an empty string.". That didn't seem to match the code, which always passed back something. I've changed the function to take a length parameter, just assuming its long enough makes me uneasy. So now if less bytes are passed in, it returns the empty string the comment promises.  Also, moved to net_utils as fmt_mac().
{quote}

Ah, yes. I changed the function and forgot to update the comment.

{quote}
A note for the future: you changed quite a bit of white space/formatting in existing code, which makes reading the diff very hard. Please try to keep things unmodified where you don't make changes.
{quote}

Sorry about that. There seem to be several indentation styles floating around the code, and I've been trying to standardize on the style that new code seems to be written in (hard tabbed [Whitesmiths|https://en.wikipedia.org/wiki/Indent_style#Whitesmiths_style]). I can definitely see how that makes diffs harder, though. Would it be ok for me to have a single commit with just the whitespace changes, which could then be checked with {{git diff --ignore-all-space}}? Or should I just leave it alone?
                
> Merge request for DHCP analyzer
> -------------------------------
>
>                 Key: BIT-1050
>                 URL: https://bro-tracker.atlassian.net/browse/BIT-1050
>             Project: Bro Issue Tracker
>          Issue Type: Improvement
>          Components: Bro
>    Affects Versions: 2.2
>            Reporter: Vlad Grigorescu
>            Assignee: Seth Hall
>              Labels: analyzer
>
> topic/vladg/dhcp is ready to go. I've been running it in prod with no problems.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://bro-tracker.atlassian.net/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira


More information about the bro-dev mailing list