script to check sequence number in a binary file

Question

This is a script to check the sequence number (8 bit unsigned integer) in a binary file and make sure it is incrementing till the end and not missing any sequence number in between. Sequence number is some 'x' bytes from a sentinel found in the binary file. The fixed size packet (some audio data) in binary file gets repeated and this aforementioned packet also has the sentinel along with other audio data.

I am new to python so I'd appreciate a code review.

#!/usr/bin/python
import sys,getopt

def parse_arguments():
  def usage():
    print("python script to check missing sequence number in a binary file")
    print("pass filename, packet_size(optional) and sequence offset where sequence number is found in the packet")
    print("python %s -f filename -s packet_size -i sequence_offset"% (sys.argv[0]))

  filename, packet_size, sequence_offset = None, 0, None
  try:
      opts,args = getopt.getopt(sys.argv[1:],'ha:f:i:s:', ['help', 'filename', 'index'])
  except getopt.GetoptError as err:
      print(err)
      sys.exit(2)

  if len(opts) != 0:
     for (o, a) in opts:
        if o in ('-h', '--help'):
           usage()
           sys.exit()
        elif o in ('-f', '--filename'):
           filename = a
        elif o in ('-s', '--packet_size'):
           packet_size = int(a)
        elif o in ('-i', '--sequence_offset'):
           sequence_offset = int(a)
        else:
           usage()
           sys.exit(2)
  else:
     usage()
     sys.exit(2)
  return filename, packet_size, sequence_offset 


def is_sequence_missing(filename, packet_size = 5392, sequence_offset = 11):
  def get_sequence_idx(filename, sentinel = ['0x4f', '0x5f', '0x4d', '0x41']):
    blob = None
    with open(filename,"rb") as f:
      blob = f.read()
      len_sentinel = len(sentinel)
      for i in range(len(blob)):
        if i + len_sentinel < len(blob) and sentinel == [hex(ord(j)) for j in blob[i:i+len_sentinel]]:
          return (blob, i)
    return (blob, None)

  missing, start, counter = False, True, 0

  blob, idx = get_sequence_idx(filename)
  if not idx:
    print("sentinel not found")
    return not missing
  print("sentinel starting at 0x%x"% (idx))

  for i in range(idx - sequence_offset, len(blob), packet_size):
    v = ord(blob[i])
    if start:
      print("started with sequence number 0x%x"% (v))
      start = False
      counter = v + 1
      continue
    if counter == v:
      counter += 1
      if counter == 256:
        counter = 0
    else:
      print("not matching expected: 0x%x receieved: 0x%x at 0x%x"% (counter, v, i))
      counter = v + 1
      missing = True
  return missing

if __name__ == "__main__":
  filename, packet_size, sequence_offset = parse_arguments()
  print("passed filename %s packet_size %d sequence_offset %d"% (filename, packet_size, sequence_offset))
  if is_sequence_missing(filename, packet_size, sequence_offset) == True:
    print("sequence is missing")
  else:
    print("sequence is not missing")

Answer 1

This answer covers the algorithmic aspect of the code. If you feel this second answer goes against the spirit of the Stack Exchange network, consider voting on the first answer instead or leaving a comment to express your judgement.

---

The algorithm

Since your question to me seems a bit vague on some parts of the data, there is a little bit of guesswork involved in the following sections, so take them with a grain of salt. I think they will nevertheless help you get an idea what to look for.

get_sequence_idx

If I understood the intent of the code correctly, its purpose is to look for some kind of sentinel value in binary data to tell where the sequence started. For that purpose, you iterate over your input file byte by byte, convert a slice of sentinel's byte length to its hex representation and compare it against the sentinel. That seems a little bit overkill to me. You will have to check this, but I think the following piece of code is actually functionaly equivalent:

def get_sequence_idx(filename, sentinel="\x4f\x5f\x4d\x41"):
    """Read the binary data and find the position of the sentinel

    This function returns the binary data as present in the file as well as
    the sentinel's (given as byte-string) position in the file. If the
    sentinel value was not found, the second return value is None.
    """
    with open(filename, "rb") as f:
        blob = f.read()
        idx = blob.find(sentinel)
        return blob, idx if idx > 0 else None

So what has happend here? I converted sentinel from a list of hexadecimal values to a "byte"-string (the term is a bit inaccurate in Python 2)¹. That allows us to use .find(...) on the input data to look for the sequence instead of doing it manually. .find(...) returns the index of the first instance where the search string was found, and -1 if it was not found. So if you were to return -1 instead of None in case of no sentinel value, the little ternary expression would not be necessary and you could just return blob, blob.find(sentinel). Since you did not provide test data you will have to verify yourself that it's actually working in your specific use-case.

is_sequence_missing

Now let's look at is_sequence_missing piece by piece:

missing, start, counter = False, True, 0

cries "C code"! That aspect was already covered in the general recommendations section of the previous answer.

if not idx:
    print("sentinel not found")
return not missing

Since you actually want to take action in case idx has the value None, you should likely also do that if idx is None:. This is also in-line with the Programming Recommendations section of PEP8, which recommends to compare singleton values (like None) using is. The intent of return not missing can also be made more clear simply by doing return True. This also allows you to narrow down the scope of missing since it will only ever be needed if you pass this initial check.

The rest of the code is IMHO mostly as straightforward as it gets, so there are only a few minor points that may improve the readability of your code. First, you can get rid of the continue if you use if ...: elif ...: else: instead of if ...: if ...: else:. If you stick with your current branch structure, putting a blank line after the first if statement can make it clearer that this is actually a seperate block that is not to be seen as part of the later "condition chain". Regarding the else branch: I think here is a opportunity to stop the loop early. If I'm not mistaken, there is no chance missing will ever become False again once you have set it to True. So if you don't need to have all the discontinued sequence numbers to be printed to stdout, a simple break there would allow the loop to end the first time you entered the else branch. In that case missing could also be omitted altogether to be replaced by return True in the else branch and by return False at the end of the function.

if __name__ == "__main__":

Using if __name__ == "__main__": is considered good practice in Python to clearly mark what code is supposed to be run if the file is used as a script. Often, programmers go even further and wrap the code that is surrounded by if __name__ == "__main__": into a main() function. That way no variables pollute the global namespace if you run the script in an interactive Python interpreter.

def main():
    filename, packet_size, sequence_offset = parse_arguments()
    print("passed filename %s packet_size %d sequence_offset %d" %
          (filename, packet_size, sequence_offset))
    if is_sequence_missing(filename, packet_size, sequence_offset):
        print("sequence is missing")
    else:
        print("sequence is not missing")


if __name__ == "__main__":
    main()

---

¹ Python 3 is more specific in that regard so there is a chance that you will end up with something like sentinel=b"\x4f\x5f\x4d\x41" or some encoding/decoding.

Answer 2

Style

Python comes with an official style guide (often just called PEP8) which is commonly accepted among Python programmers. Since you said you are new to Python, I wholeheartedly recommend to read and follow it.

E.g. one of the recommendations of the style guide is to indent code by 4 spaces per indentation level, while you have a mix of 2 and four spaces. Even if you decide to stick with 2 spaces: be consistent!

Another very widely accepted convention is to have no whitespace around = if used in keyword-arguments to functions, e.g. instead of

def is_sequence_missing(filename, packet_size = 5392, sequence_offset = 11): 

use

def is_sequence_missing(filename, packet_size=5392, sequence_offset=11):

Outdated Python

Your code uses some parts of the Python language which have become a bit "rusty", i.e. have been superseded in later versions of the language. Especially in the light of soon to come end-of-life of Python 2.7, I would highly recommend to have a look at Python 3 or at least use "more current" features of Python 2.7.

A few examples:

Command-line arguments

Your code uses getopt to parse command-line arguments. getopt has been superseded by optparse, which in turn has been marked as deprecated starting from Python 2.7 to be replaced by argparse (see also this SO post from 2010!) from there on. Apart from being easier to use, argparse will e.g. also generate and show usage instructions if the user fails to give the correct arguments.

Further reading: The official and reasonably comprehensive tutorial on argparse.

String formatting

String output is generated using the old % formatting. As of Python 2.6, the recommended way is to use str.format for string formatting. If you happen to have access to Python 3.6 and later, also so called f-strings are available to you which make the whole thing even more comfortable. An example from your code:

"not matching expected: 0x%x received: 0x%x at 0x%x"% (counter, v, i)
"not matching expected: 0x{:x} received: 0x{:x} at 0x{:x}".format(counter, v, i)  # Python 2.6+
f"not matching expected: 0x{counter:x} received: 0x{v:x} at 0x{i:x}"  # Python 3.6+

Further reading: This blog post has a nice comparison of all three variants and their possibilities.

General recommendations

Variable definition

Usually it's always a good idea to define your variables as close as possible to where they are needed, and also arguably define one variable per line. This helps to avoid unused or uninitialized variables, although the later part is not so critical here in Python as it would be e.g. in C or C++.

Documentation

Also not strictly specific to Python: Do yourself (and possible also others) a favor and document your code. Maybe in a few months time where you have done something different you will (have to) come back and wonder why the default value for sequence_offset is 11. Python has a feature called docstrings which are also described in the style guide (and in more detail in PEP 257) and are very commonly found in Python code. docstrings are essentially """triple quoted strings""" placed immediately after the function definition, trying to describe what to expect from the function. As an example:

def is_sequence_missing(filename, packet_size=5392, sequence_offset=11):
    """Check file for gaps in the sequence number

    read up binray data from filename and check for ascending
    sequence numbers starting sequence_offset (default: 11) bytes away
    from the sentinel value
    """
    # rest of the code here

Documentation written using docstrings will be picked up by Python's built-in help(...) as well as by most Python IDEs.

---

That's all for now. Maybe I can spare some time tomorrow to have a closer look at the actual algorithm. Edit: See the second answer for a more detailed feedback on the actual algorithm.