[Bro-Dev] Some bro.bif issues

[Matthias Vallentin]

This is the function set_buf:

    function set_buf%(f: file, buffered: bool%): any
        %{
        f->SetBuf(buffered);
        return new Val(0, TYPE_VOID);
        %}

It's return value is any although void is returned. Shouldn't the return value
in the function signature be void as well?

Moreover, here is the function exit:

    function exit%(%): int
        %{
        exit(0);
        return 0;
        %}

Shouldn't it return 'void' at the scripting layer since it shuts down
the Bro process immediately? Why not let users control the return code
like this:

    function exit%(code: int): void
        %{
        exit(code);
        %}

There seems to be another redundancy:

    function active_connection%(id: conn_id%): bool
        %{
        Connection* c = sessions->FindConnection(id);
        return new Val(c ? 1 : 0, TYPE_BOOL);
        %}

and

    function connection_exists%(c: conn_id%): bool
        %{
        if ( sessions->FindConnection(c) )
            return new Val(1, TYPE_BOOL);
        else
            return new Val(0, TYPE_BOOL);
        %}

are essentially the same. Only the latter is used in the current set of
scripts. Should we get rid of the former?

The same sort of redundancy exists for:

    function connection_record%(cid: conn_id%): connection
        %{
        Connection* c = sessions->FindConnection(cid);
        if ( c )
            return c->BuildConnVal();
        else
            {
            // Hard to recover from this until we have union types ...
            builtin_error("connection ID not a known connection (fatal)", cid);
            exit(0);
            return 0;
            }
        %}

and

    function lookup_connection%(cid: conn_id%): connection
        %{
        Connection* conn = sessions->FindConnection(cid);
        if ( conn )
            return conn->BuildConnVal();

        builtin_error("connection ID not a known connection", cid);

        // Return a dummy connection record.
        RecordVal* c = new RecordVal(connection_type);

        RecordVal* id_val = new RecordVal(conn_id);
        id_val->Assign(0, new AddrVal((unsigned int) 0));
        id_val->Assign(1, new PortVal(ntohs(0), TRANSPORT_UDP));
        id_val->Assign(2, new AddrVal((unsigned int) 0));
        id_val->Assign(3, new PortVal(ntohs(0), TRANSPORT_UDP));
        c->Assign(0, id_val);

        RecordVal* orig_endp = new RecordVal(endpoint);
        orig_endp->Assign(0, new Val(0, TYPE_COUNT));
        orig_endp->Assign(1, new Val(int(0), TYPE_COUNT));

        RecordVal* resp_endp = new RecordVal(endpoint);
        resp_endp->Assign(0, new Val(0, TYPE_COUNT));
        resp_endp->Assign(1, new Val(int(0), TYPE_COUNT));

        c->Assign(1, orig_endp);
        c->Assign(2, resp_endp);

        c->Assign(3, new Val(network_time, TYPE_TIME));
        c->Assign(4, new Val(0.0, TYPE_INTERVAL));
        c->Assign(5, new TableVal(string_set)); // service
        c->Assign(6, new StringVal(""));    // addl
        c->Assign(7, new Val(0, TYPE_COUNT));   // hot
        c->Assign(8, new StringVal(""));    // history

        return c;
        %}

except that the latter returns a dummy connection value. Neither function is
currently used at script land. My vote would be to remove the former one, as
the latter has more graceful semantics.

    Matthias