View Single Post
Old 2011-06-12, 09:57   #3
D. B. Staple
 
D. B. Staple's Avatar
 
Nov 2007
Halifax, Nova Scotia

23×7 Posts
Default Bug fix

Hello everyone,

It looks like I found a bug in factmsieve.py. I don't know why this bug is only showing itself on my system; presumably there's something strange going on on my system causing the python script to be abused, causing the bug to show itself. Anyway, here's the fix:

I suggest to replace this code:
Code:
def read_spq(fact_p):
  for j in range(SV_THREADS):
    ql, qp, qh = fact_p['q_dq'][j]
    try:
      with open('.last_spq' + str(100 * PNUM + j), 'r') as in_f:
        tmp = remove_ws(chomp(in_f.readline()))
        if tmp:
          t = int(chomp(tmp))
          if t > qp:
            fact_p['q_dq'][j] = (ql, t, qh)
    except IOError:
      pass
With this modified version:
Code:
def read_spq(fact_p):
  for j in range(SV_THREADS):
    ql, qp, qh = fact_p['q_dq'][j]
    try:
      with open('.last_spq' + str(100 * PNUM + j), 'r') as in_f:
        tmp = in_f.readline()
        try:
          t = int(tmp)
        except ValueError:
          pass
        else:
          if t > qp:
            fact_p['q_dq'][j] = (ql, t, qh)
    except IOError:
      pass
To be honest I don't completely understand the crash of the original code. Ultimately, chomp(tmp) sometimes returns an empty string, causing int() to throw a ValueException. This doesn't make sense to me, because if tmp is empty then it should not pass the line containing 'if tmp:'. Perhaps tmp is sometimes composed entirely of line breaks, such that tmp is nonempty but chomp(tmp) is an empty string. However, this doesn't make sense, because remove_ws should ensure that tmp contains no linebreaks. So ultimately I don't yet understand how the original crash occurs; perhaps there is a bug in chomp or remove_ws that I haven't found. My impression is that the problem gets worse with a larger number of threads, so perhaps it's related to locking .last_spq or something of that nature.

In any case, it seems to me that these few lines are not robustly written and can be improved.
Firstly, why do we call chomp() twice when remove_ws() is called? remove_ws will also remove line breaks and carriage returns.
Secondly, python has built-in functions for removing whitespace, they're called strip, lstrip, rstrip, etc., so one should probably not write his own. Try this at a Python terminal:
Code:
temp='\n\n\r\r\t\t 14  \n\n\r'
temp.strip()
Thirdly, int() is already coded to remove whitespace, see http://docs.python.org/library/functions.html#int , so actually whitespace removal before calling int() is redundant.
Finally, why not simply add try/except to int()? That way the string is only parsed if it can give a sensible result.

In the end I suggest modifying the code as described above, which solves the issue and allows me to run crash-free. However, I'm not an expert python programmer, so maybe it's not perfect. If anyone is interested then perhaps we can try to nail down the error further.
D. B. Staple is offline   Reply With Quote