Bug #94

src/gmxlib/libxdrf.c is not robust enough

Added by LI Daobing about 14 years ago. Updated almost 14 years ago.

Target version:
Affected version - extra info:
Affected version:


libxdrf.c is not robust enough on XTC_MAGIC, some time it fault, some time it
return a wrong value, some time it HANG!(for example, in get_current_frame_time)

I will give a patch in attachment

xdrf.patch (5.75 KB) xdrf.patch a patch LI Daobing, 07/30/2006 11:53 AM
2.xtc (6.69 KB) 2.xtc 2,xtc (testcase: xtc file) LI Daobing, 08/12/2006 01:21 PM
2.pdb.bz2 (3.81 KB) 2.pdb.bz2 2.pdb.bz2 (source of the xtc testcase) LI Daobing, 08/12/2006 01:22 PM
libxdrf.patch (7.43 KB) libxdrf.patch libxdrf Patch Filipe Maia, 08/22/2006 04:51 PM


#1 Updated by LI Daobing about 14 years ago

Created an attachment (id=45)
a patch

#2 Updated by Filipe Maia about 14 years ago

All these patches only fix the read of broken xtc files (for exampes files that
have incomplete headers, having for example 2 magic numbers one next to each
other). If we want to take into account broken xtc files, many more things have
to be changed. If there is a case of a legitimate xtc file that works with this
code but doesn't work with the current one I would like to take a look at it.
Thanks for the patch :-), but I won't apply it yet, without a test case.

#3 Updated by LI Daobing about 14 years ago

the developers have discovered this bug in 2005, check it at [1].

anyway, I will provide a testcase for this in this weekend, for example, an xtc
file with 1995 atoms.

$ cat -n trxio.c | dog -l 625-638
625 case efXTC:
626 /* B. Hess 2005-4-20
627 * Sometimes is off by one frame
628 * and sometimes reports frame not present/file not seekable
629 /
630 /
DvdS 2005-05-31: this has been fixed along with the increased
631 * accuracy of the control over -b and -e options.
632 */
633 if (bTimeSet(TBEGIN) && (fr->time < rTimeValue(TBEGIN))) {
634 if (xtc_seek_time(rTimeValue(TBEGIN),status,fr->natoms)) {
635 gmx_fatal(FARGS,"Specified frame doesn't exist or file
not seekable");
636 }
638 }

#4 Updated by LI Daobing almost 14 years ago

Created an attachment (id=50)
testcase: xtc file

#5 Updated by LI Daobing almost 14 years ago

Created an attachment (id=51)
source of the xtc testcase

#6 Updated by LI Daobing almost 14 years ago

a testcase for this bug:

2.pdb is a pdb file contain 1995 atoms (in attachment 2.pdb.bz2), convert 2.pdb
to 2.xtc with command [1](2.xtc also in attachment), then run2, you will find
this program is hang and eat all CPU time.

$ trjcat -f 2.pdb -o 2.xtc

$ trjconv -f 2.xtc -b 1

#7 Updated by David van der Spoel almost 14 years ago

This is a pathetic example because the number of atoms is 1995. Obviously the
algorithm may crash. I agree that the library is not robust enough, but making a
complicated patch for systems with just this number of atoms is unreasonable.

Please provide a different test case...

#8 Updated by Filipe Maia almost 14 years ago

You are correct about the problems when handlings systems with 1995 atoms.
There are actually other problems that your patch doesn't address like 1995
atoms at timestep 1995. There are even problems with atoms with different
number of atoms like timestep 1995 and a floating point representation of time
that when interpreted as int reads 1995. I'll see if I can address all this
using the current format. If nothing works, then maybe changing the magic
number to a negative number could be a possibility (which at the moment seems
to fix most of the problems, if not all).

#9 Updated by Filipe Maia almost 14 years ago

Created an attachment (id=63)
libxdrf Patch

I think i managed to solve all the ambiguities.
If no one complaints i'm gonna commit the patch to CVS in the next few days and
mark the bug as fixed.

#10 Updated by Filipe Maia almost 14 years ago

Patch commited.

Also available in: Atom PDF