US Software Capstone Design Project

GNAT - Graphical Network Analysis Tool

Code Inspection Report #1

 

 

Design Team:

Matt Ericson

John Kobinski

Matt Petro

Rob Waltz

 

 

Client:

Rich Ames, US Software

 

 

 

 

v1.0

April 2, 1998

Last revision: 4/2/98

TABLE OF CONTENTS

EXECUTIVE SUMMARY *

CODE INSPECTION SUMMARY REPORTS *

SOURCE CODE LISTING *

DESCRIPTION OF REVIEWED CODE *

PREPARATION AND REVIEW TIME *

KEY FINDINGS *

REVIEW OF THE REVIEW *

 

 

EXECUTIVE SUMMARY

 

This document summarizes the results of the GNAT design team’s first formal code review. The participants in the review were the design team and Rich Ames and the code which was reviewed was written by Matt Ericson. The code primarily consisted of the data structures which hold lists of packets. These data structures will be used in the program to generate useful statistics. For more information, consult the GNAT Design Document and Specifications Document.

 

Contained in this report are Code Inspection Summary Report Forms from all four team members, a listing of all reviewed source code, a description of the code’s functionality and a summary of the reviews findings.

 

CODE INSPECTION SUMMARY REPORTS

 

Attached at end of document.

 

SOURCE CODE LISTING

 

Attached at end of document.

 

DESCRIPTION OF REVIEWED CODE

 

Classes Reviewed:

PacketReference

Connection

ConnectionList

PacketList

Packet

RetransmissionList

ZeroPacketList

 

All of the Classes reviewed are have attributes which are data types that contain data for the program.

 

Connection is the class that keeps track of all the connections. This class is a linked list of ConnectionLists. Each node in this list represents a connection between two computers. This class has functionality that allows it to find the connection on which a given packet was sent.

 

ConnectionList is the class that keeps track of all of the packets which were sent in one direction on a connection. It is also a node in the linked list maintained by the class Connection. ConnectionList has functionality that allows it to find out if a particular packet contains data which was sent on this connection previously (ie: retransmitted data).

 

Packet is the abstract data type that will contain all data from a specific packet. This is set up as a node in a linked list. It is the only class that has actual data from a packet in the Snoop file. It has all fields that we need to do any calculations that the user might want.

 

PacketList this class is the class that actually contains all the Packets that were parsed from a Snoop file. This is just a linked list of Packet objects.

 

PacketReference is a simple class that contains only a constructor. The purpose of this class is to be a node in a linked list and have a reference to a real Packet.

 

RetransmissionList contains a linked list of PacketReferences. The PacketReferences will refer to Packets in PacketLIst which were later retransmitted. This list will have to be constructed after all the other lists are constructed so there is a special function for that.

 

ZeroPacketList has a similar structure to RetransmissionList. It will contain a linked list of PacketReferences, but they will refer to Packets which have window sizes of 0.

 

PREPARATION AND REVIEW TIME

 

Team member preparation time:

Matt Ericson n/a

John Kobinski 45 min.

Matt Petro 60 min.

Rob Waltz 50 min.

 

Total time for review: 60 min.

 

KEY FINDINGS

 

  1. Use more descriptive class and varaible names, specifically change class "Connection."
  2.  

  3. New method for calculating retransmissions will necessitate adding or modifying functionality of the "find_seq" function.
  4.  

  5. Throughout the code the counter variable "i" is used superfluously.
  6.  

  7. Place pieces of code that are useful for troubleshooting but inappropriate for release in areas that can be left out of compilation using the appropriate switch.
  8.  

  9. Include the transport protocol (either UDP or TCP) in functionality.
  10.  

  11. Correct spelling on line 71 of "PacketList".
  12.  

  13. Remove "time_of_max_frame_rate" and "num_of_connections" from PacketList line 30.
  14.  

  15. Use "if" statement to avoid checking for a packet index number that is greater than the total number of packets.
  16.  

  17. Add boolean "checksum" member to packet class.

 

REVIEW OF THE REVIEW

 

We felt that the review was fairly effective and useful for three main reasons. First, it was helpful for us to review Matt Ericson’s code, as the rest of us could get an update on exactly how his code was implemented. This gave us a good understanding of how his area of GNAT worked. Future code reviews will do doubt prove useful, as we will all gain an understanding of how everyone else’s code works.

 

Second, the review was useful because our sponsor, Rich Ames, was able to gain an understanding of how we were implementing our design. Although Rich was familiar with our OMT diagrams and overall design, it was useful for him to see actual code which implemented the design. So, in addition to helping our understanding of the implementation, the review helped Rich as well.

 

Finally, this review was useful because Rich was able to point out some areas where our algorithms were incorrect. He noticed that our algorithm for detecting retransmissions could miss some retransmissions due to differences in sequence numbers of retransmitted data. Also, he was able to suggest some ways for us to correct the algorithm and we plan to incorporate these changes into our final code.