Wireshark · Wireshark-bugs: [Wireshark-bugs] [Bug 4189] New: Wireshark code indenting

[bugzilla-daemon@xxxxxxxxxxxxx]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=4189

           Summary: Wireshark code indenting
           Product: Wireshark
           Version: 1.3.x (Experimental)
          Platform: Other
        OS/Version: All
            Status: NEW
          Severity: Major
          Priority: Low
         Component: TShark
        AssignedTo: wireshark-bugs@xxxxxxxxxxxxx
        ReportedBy: nepenthesdev@xxxxxxxxx



Markus <nepenthesdev@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3879|                            |review_for_checkin?
               Flag|                            |


Created an attachment (id=3879)
 --> (https://bugs.wireshark.org/bugzilla/attachment.cgi?id=3879)
example configuration for uncrustify.sourceforge.net

Build Information:
TShark 1.3.0

Copyright 1998-2009 Gerald Combs <gerald@xxxxxxxxxxxxx> and contributors.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Compiled with GLib 2.20.1, with libpcap 1.0.0, with libz 1.2.3.3, without POSIX
capabilities, with libpcre 7.8, without SMI, without c-ares, with ADNS, without
Lua, without Python, without GnuTLS, without Gcrypt, with MIT Kerberos, without
GeoIP.

Running on Linux 2.6.28-16-generic, with libpcap version 1.0.0.

Built using gcc 4.4.0 20090419 (prerelease) [gcc-4_4-branch revision 146360].

--
You should have a look on uncrustify.sourceforge.net, create a config file for
it, and auto-indent the code.
Current 'indenting' makes it very hard to spot read/spot bugs.

example:
epan/dissectors/packet-dcerpc.c
---
static void
dissect_dcerpc_cn_bind (tvbuff_t *tvb, gint offset, packet_info *pinfo,
                        proto_tree *dcerpc_tree, e_dce_cn_common_hdr_t *hdr)
{
    conversation_t *conv = NULL;
    guint8 num_ctx_items = 0;
    guint i;
    gboolean saw_ctx_item = FALSE;
    guint16 ctx_id;
    guint8 num_trans_items;
    guint j;
    e_uuid_t if_id;
    e_uuid_t trans_id;
    guint32 trans_ver;
    guint16 if_ver, if_ver_minor;
    dcerpc_auth_info auth_info;
    char *uuid_str;
    const char *uuid_name = NULL;
    proto_item *iface_item = NULL;

    offset = dissect_dcerpc_uint16 (tvb, offset, pinfo, dcerpc_tree, hdr->drep,
                                    hf_dcerpc_cn_max_xmit, NULL);

    offset = dissect_dcerpc_uint16 (tvb, offset, pinfo, dcerpc_tree, hdr->drep,
                                    hf_dcerpc_cn_max_recv, NULL);

    offset = dissect_dcerpc_uint32 (tvb, offset, pinfo, dcerpc_tree, hdr->drep,
                                    hf_dcerpc_cn_assoc_group, NULL);

    offset = dissect_dcerpc_uint8 (tvb, offset, pinfo, dcerpc_tree, hdr->drep,
                                    hf_dcerpc_cn_num_ctx_items,
&num_ctx_items);

    /* padding */
    offset += 3;

    for (i = 0; i < num_ctx_items; i++) {
            proto_item *ctx_item = NULL;
            proto_tree *ctx_tree = NULL, *iface_tree = NULL;
        gint ctx_offset = offset;

      dissect_dcerpc_uint16 (tvb, offset, pinfo, NULL, hdr->drep,
                                      hf_dcerpc_cn_ctx_id, &ctx_id);

      /* save context ID for use with dcerpc_add_conv_to_bind_table() */
      /* (if we have multiple contexts, this might cause "decode as"
       *  to behave unpredictably) */
      pinfo->dcectxid = ctx_id;

      if (dcerpc_tree) {
              ctx_item = proto_tree_add_item(dcerpc_tree,
hf_dcerpc_cn_ctx_item,
                                             tvb, offset, 0,
                                             hdr->drep[0] & 0x10);
              ctx_tree = proto_item_add_subtree(ctx_item, ett_dcerpc_cn_ctx);
      }

      offset = dissect_dcerpc_uint16 (tvb, offset, pinfo, ctx_tree, hdr->drep,
                                      hf_dcerpc_cn_ctx_id, &ctx_id);
      offset = dissect_dcerpc_uint8 (tvb, offset, pinfo, ctx_tree, hdr->drep,
                                      hf_dcerpc_cn_num_trans_items,
&num_trans_items);

      if(dcerpc_tree) {
        proto_item_append_text(ctx_item, "[%u]: ID:%u", i+1, ctx_id);
      }

      /* padding */
      offset += 1;

      dcerpc_tvb_get_uuid (tvb, offset, hdr->drep, &if_id);
      if (ctx_tree) {

      iface_item = proto_tree_add_item(ctx_tree,
hf_dcerpc_cn_bind_abstract_syntax, tvb, offset, 0, FALSE);
          iface_tree = proto_item_add_subtree(iface_item, ett_dcerpc_cn_iface);

      uuid_str = guid_to_str((e_guid_t*)&if_id);
      uuid_name = guids_get_uuid_name(&if_id);
      if(uuid_name) {
                  proto_tree_add_guid_format (iface_tree,
hf_dcerpc_cn_bind_if_id, tvb,
                                        offset, 16, (e_guid_t *) &if_id,
"Interface: %s UUID: %s", uuid_name, uuid_str);
          proto_item_append_text(iface_item, ": %s", uuid_name);
      } else {
          proto_tree_add_guid_format (iface_tree, hf_dcerpc_cn_bind_if_id, tvb,
                                        offset, 16, (e_guid_t *) &if_id,
"Interface UUID: %s", uuid_str);
          proto_item_append_text(iface_item, ": %s", uuid_str);
      }
      }
      offset += 16;

      if (hdr->drep[0] & 0x10) {
          offset = dissect_dcerpc_uint16 (tvb, offset, pinfo, iface_tree,
hdr->drep,
                                          hf_dcerpc_cn_bind_if_ver, &if_ver);
          offset = dissect_dcerpc_uint16 (tvb, offset, pinfo, iface_tree,
hdr->drep,
                                          hf_dcerpc_cn_bind_if_ver_minor,
&if_ver_minor);
      } else {
          offset = dissect_dcerpc_uint16 (tvb, offset, pinfo, iface_tree,
hdr->drep,
                                          hf_dcerpc_cn_bind_if_ver_minor,
&if_ver_minor);
          offset = dissect_dcerpc_uint16 (tvb, offset, pinfo, iface_tree,
hdr->drep,
                                          hf_dcerpc_cn_bind_if_ver, &if_ver);
      }

      if (ctx_tree) {
          proto_item_append_text(iface_item, " V%u.%u", if_ver, if_ver_minor);
          proto_item_set_len(iface_item, 20);
      }

      if (!saw_ctx_item) {
        conv = find_conversation (pinfo->fd->num, &pinfo->src, &pinfo->dst,
pinfo->ptype,
                                  pinfo->srcport, pinfo->destport, 0);
        if (conv == NULL) {
            conv = conversation_new (pinfo->fd->num, &pinfo->src, &pinfo->dst,
pinfo->ptype,
                                     pinfo->srcport, pinfo->destport, 0);
        }


        /* if this is the first time we see this packet, we need to
           update the dcerpc_binds table so that any later calls can
           match to the interface.
           XXX We assume that BINDs will NEVER be fragmented.
        */
        if(!(pinfo->fd->flags.visited)){
                dcerpc_bind_key *key;
                dcerpc_bind_value *value;

                key = se_alloc (sizeof (dcerpc_bind_key));
                key->conv = conv;
                key->ctx_id = ctx_id;
                key->smb_fid = dcerpc_get_transport_salt(pinfo);

                value = se_alloc (sizeof (dcerpc_bind_value));
                value->uuid = if_id;
                value->ver = if_ver;

                /* add this entry to the bind table, first removing any
                   previous ones that are identical
                 */
                if(g_hash_table_lookup(dcerpc_binds, key)){
                        g_hash_table_remove(dcerpc_binds, key);
                }
                g_hash_table_insert (dcerpc_binds, key, value);
        }

        if (check_col (pinfo->cinfo, COL_INFO)) {
          if (num_ctx_items > 1)
                  col_append_fstr(pinfo->cinfo, COL_INFO, ", %u context items,
1st", num_ctx_items);

                col_append_fstr(pinfo->cinfo, COL_INFO, " %s V%u.%u",
                       guids_resolve_uuid_to_str(&if_id), if_ver,
if_ver_minor);
        }
        saw_ctx_item = TRUE;
      }

      for (j = 0; j < num_trans_items; j++) {
            proto_tree *trans_tree = NULL;
            proto_item *trans_item = NULL;

        dcerpc_tvb_get_uuid (tvb, offset, hdr->drep, &trans_id);
        if (ctx_tree) {

        trans_item = proto_tree_add_item(ctx_tree,
hf_dcerpc_cn_bind_trans_syntax, tvb, offset, 0, FALSE);
        trans_tree = proto_item_add_subtree(trans_item,
ett_dcerpc_cn_trans_syntax);

        uuid_str = guid_to_str((e_guid_t *) &trans_id);
            proto_tree_add_guid_format (trans_tree, hf_dcerpc_cn_bind_trans_id,
tvb,
                                          offset, 16, (e_guid_t *) &trans_id,
"Transfer Syntax: %s", uuid_str);
            proto_item_append_text(trans_item, "[%u]: %s", j+1, uuid_str);
        }
        offset += 16;

        offset = dissect_dcerpc_uint32 (tvb, offset, pinfo, trans_tree,
hdr->drep,
                                        hf_dcerpc_cn_bind_trans_ver,
&trans_ver);
        if (ctx_tree) {
          proto_item_set_len(trans_item, 20);
          proto_item_append_text(trans_item, " V%u", trans_ver);
        }
      }

      if(ctx_tree) {
        proto_item_set_len(ctx_item, offset - ctx_offset);
      }
    }

    /*
     * XXX - we should save the authentication type *if* we have
     * an authentication header, and associate it with an authentication
     * context, so subsequent PDUs can use that context.
     */
    dissect_dcerpc_cn_auth (tvb, offset, pinfo, dcerpc_tree, hdr, TRUE,
&auth_info);
}
------

/opt/uncrustify/bin/uncrustify -c /tmp/uncrustify.conf --replace
epan/dissectors/packet-dcerpc.c

--------
static void
dissect_dcerpc_cn_bind (tvbuff_t *tvb, gint offset, packet_info *pinfo,
proto_tree *dcerpc_tree, e_dce_cn_common_hdr_t *hdr)
{
        conversation_t *conv = NULL;
        guint8 num_ctx_items = 0;
        guint i;
        gboolean saw_ctx_item = FALSE;
        guint16 ctx_id;
        guint8 num_trans_items;
        guint j;
        e_uuid_t if_id;
        e_uuid_t trans_id;
        guint32 trans_ver;
        guint16 if_ver, if_ver_minor;
        dcerpc_auth_info auth_info;
        char *uuid_str;
        const char *uuid_name = NULL;
        proto_item *iface_item = NULL;

        offset = dissect_dcerpc_uint16(tvb, offset, pinfo, dcerpc_tree,
hdr->drep,
                                                                  
hf_dcerpc_cn_max_xmit, NULL);

        offset = dissect_dcerpc_uint16(tvb, offset, pinfo, dcerpc_tree,
hdr->drep,
                                                                  
hf_dcerpc_cn_max_recv, NULL);

        offset = dissect_dcerpc_uint32(tvb, offset, pinfo, dcerpc_tree,
hdr->drep,
                                                                  
hf_dcerpc_cn_assoc_group, NULL);

        offset = dissect_dcerpc_uint8(tvb, offset, pinfo, dcerpc_tree,
hdr->drep,
                                                                 
hf_dcerpc_cn_num_ctx_items, &num_ctx_items);

        /* padding */
        offset += 3;

        for (i = 0; i < num_ctx_items; i++)
        {
                proto_item *ctx_item = NULL;
                proto_tree *ctx_tree = NULL, *iface_tree = NULL;
                gint ctx_offset = offset;

                dissect_dcerpc_uint16(tvb, offset, pinfo, NULL, hdr->drep,
                                                          hf_dcerpc_cn_ctx_id,
&ctx_id);

                /* save context ID for use with dcerpc_add_conv_to_bind_table()
*/
                /* (if we have multiple contexts, this might cause "decode as"
                 *  to behave unpredictably) */
                pinfo->dcectxid = ctx_id;

                if (dcerpc_tree)
                {
                        ctx_item = proto_tree_add_item(dcerpc_tree,
hf_dcerpc_cn_ctx_item,
                                                                               
   tvb, offset, 0,
                                                                               
   hdr->drep[0] & 0x10);
                        ctx_tree = proto_item_add_subtree(ctx_item,
ett_dcerpc_cn_ctx);
                }

                offset = dissect_dcerpc_uint16(tvb, offset, pinfo, ctx_tree,
hdr->drep,
                                                                          
hf_dcerpc_cn_ctx_id, &ctx_id);
                offset = dissect_dcerpc_uint8(tvb, offset, pinfo, ctx_tree,
hdr->drep,
                                                                         
hf_dcerpc_cn_num_trans_items, &num_trans_items);

                if(dcerpc_tree)
                {
                        proto_item_append_text(ctx_item, "[%u]: ID:%u", i+1,
ctx_id);
                }

                /* padding */
                offset += 1;

                dcerpc_tvb_get_uuid(tvb, offset, hdr->drep, &if_id);
                if (ctx_tree)
                {

                        iface_item = proto_tree_add_item(ctx_tree,
hf_dcerpc_cn_bind_abstract_syntax, tvb, offset, 0, FALSE);
                        iface_tree = proto_item_add_subtree(iface_item,
ett_dcerpc_cn_iface);

                        uuid_str = guid_to_str((e_guid_t*)&if_id);
                        uuid_name = guids_get_uuid_name(&if_id);
                        if(uuid_name)
                        {
                                proto_tree_add_guid_format(iface_tree,
hf_dcerpc_cn_bind_if_id, tvb,
                                                                               
   offset, 16, (e_guid_t *) &if_id, "Interface: %s UUID: %s", uuid_name,
uuid_str);
                                proto_item_append_text(iface_item, ": %s",
uuid_name);
                        } else
                        {
                                proto_tree_add_guid_format(iface_tree,
hf_dcerpc_cn_bind_if_id, tvb,
                                                                               
   offset, 16, (e_guid_t *) &if_id, "Interface UUID: %s", uuid_str);
                                proto_item_append_text(iface_item, ": %s",
uuid_str);
                        }
                }
                offset += 16;

                if (hdr->drep[0] & 0x10)
                {
                        offset = dissect_dcerpc_uint16(tvb, offset, pinfo,
iface_tree, hdr->drep,
                                                                               
   hf_dcerpc_cn_bind_if_ver, &if_ver);
                        offset = dissect_dcerpc_uint16(tvb, offset, pinfo,
iface_tree, hdr->drep,
                                                                               
   hf_dcerpc_cn_bind_if_ver_minor, &if_ver_minor);
                } else
                {
                        offset = dissect_dcerpc_uint16(tvb, offset, pinfo,
iface_tree, hdr->drep,
                                                                               
   hf_dcerpc_cn_bind_if_ver_minor, &if_ver_minor);
                        offset = dissect_dcerpc_uint16(tvb, offset, pinfo,
iface_tree, hdr->drep,
                                                                               
   hf_dcerpc_cn_bind_if_ver, &if_ver);
                }

                if (ctx_tree)
                {
                        proto_item_append_text(iface_item, " V%u.%u", if_ver,
if_ver_minor);
                        proto_item_set_len(iface_item, 20);
                }

                if (!saw_ctx_item)
                {
                        conv = find_conversation(pinfo->fd->num, &pinfo->src,
&pinfo->dst, pinfo->ptype,
                                                                        
pinfo->srcport, pinfo->destport, 0);
                        if (conv == NULL)
                        {
                                conv = conversation_new(pinfo->fd->num,
&pinfo->src, &pinfo->dst, pinfo->ptype,
                                                                               
pinfo->srcport, pinfo->destport, 0);
                        }


                        /* if this is the first time we see this packet, we
need to
                           update the dcerpc_binds table so that any later
calls can
                           match to the interface.
                           XXX We assume that BINDs will NEVER be fragmented.
                         */
                        if(!(pinfo->fd->flags.visited))
                        {
                                dcerpc_bind_key *key;
                                dcerpc_bind_value *value;

                                key = se_alloc(sizeof(dcerpc_bind_key));
                                key->conv = conv;
                                key->ctx_id = ctx_id;
                                key->smb_fid =
dcerpc_get_transport_salt(pinfo);

                                value = se_alloc(sizeof(dcerpc_bind_value));
                                value->uuid = if_id;
                                value->ver = if_ver;

                                /* add this entry to the bind table, first
removing any
                                   previous ones that are identical
                                 */
                                if(g_hash_table_lookup(dcerpc_binds, key))
                                {
                                        g_hash_table_remove(dcerpc_binds, key);
                                }
                                g_hash_table_insert(dcerpc_binds, key, value);
                        }

                        if (check_col(pinfo->cinfo, COL_INFO))
                        {
                                if (num_ctx_items > 1)
                                        col_append_fstr(pinfo->cinfo, COL_INFO,
", %u context items, 1st", num_ctx_items);

                                col_append_fstr(pinfo->cinfo, COL_INFO, " %s
V%u.%u",
                                                               
guids_resolve_uuid_to_str(&if_id), if_ver, if_ver_minor);
                        }
                        saw_ctx_item = TRUE;
                }

                for (j = 0; j < num_trans_items; j++)
                {
                        proto_tree *trans_tree = NULL;
                        proto_item *trans_item = NULL;

                        dcerpc_tvb_get_uuid(tvb, offset, hdr->drep, &trans_id);
                        if (ctx_tree)
                        {

                                trans_item = proto_tree_add_item(ctx_tree,
hf_dcerpc_cn_bind_trans_syntax, tvb, offset, 0, FALSE);
                                trans_tree = proto_item_add_subtree(trans_item,
ett_dcerpc_cn_trans_syntax);

                                uuid_str = guid_to_str((e_guid_t *) &trans_id);
                                proto_tree_add_guid_format(trans_tree,
hf_dcerpc_cn_bind_trans_id, tvb,
                                                                               
   offset, 16, (e_guid_t *) &trans_id, "Transfer Syntax: %s", uuid_str);
                                proto_item_append_text(trans_item, "[%u]: %s",
j+1, uuid_str);
                        }
                        offset += 16;

                        offset = dissect_dcerpc_uint32(tvb, offset, pinfo,
trans_tree, hdr->drep,
                                                                               
   hf_dcerpc_cn_bind_trans_ver, &trans_ver);
                        if (ctx_tree)
                        {
                                proto_item_set_len(trans_item, 20);
                                proto_item_append_text(trans_item, " V%u",
trans_ver);
                        }
                }

                if(ctx_tree)
                {
                        proto_item_set_len(ctx_item, offset - ctx_offset);
                }
        }

        /*
         * XXX - we should save the authentication type *if* we have
         * an authentication header, and associate it with an authentication
         * context, so subsequent PDUs can use that context.
         */
        dissect_dcerpc_cn_auth(tvb, offset, pinfo, dcerpc_tree, hdr, TRUE,
&auth_info);
}
-----------

easier to read, easier to debug, easier to maintain.


You do not have to use my uncrustify settings, just use some.
I've attached my uncrustify settings as starting point, maybe it helps.


-- 
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.