[segyio] 236/376: Fix off-by-one error in non-mmap field_forall

[Jørgen Kvalsvik]

This is an automated email from the git hooks/post-receive script.

jokva-guest pushed a commit to branch debian
in repository segyio.

commit 650ac26ec2a76648826f6336fd15b2401767dd59
Author: Jørgen Kvalsvik <jokva at statoil.com>
Date:   Thu Mar 9 13:00:51 2017 +0100

    Fix off-by-one error in non-mmap field_forall
    
    Fixes an off-by-one error that wasn't caught by tests because our test
    files are mostly zero and the mis-aligned read happened to not affect
    the result.
    
    The issue stems from get_field being aware of the 'field' argument being
    1-index'd, but the preceeding fread and its buffer were not. seek+read
    is simply made aware of the 1-indexing.
    
    Our test files headers are mostly zeros. The field-forall function would
    read from byte 189, but should've started at byte 188, and the test data
    happens to have all bits in the least-significant byte. When this is
    copied to the local buffer, it would then be *written* with the same
    mis-aligned offset, and since this didn't interact with any other bytes
    (they're all set to zero anyway), the read did not pick up on it. This
    is verifyable by reading the bytes into an int and using ntohl() or by
    inspecting the memory with a debugger.
---
 lib/src/segy.c      | 9 ++++++---
 python/test/segy.py | 4 ++++
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/lib/src/segy.c b/lib/src/segy.c
index 6f3363d..39c43fb 100644
--- a/lib/src/segy.c
+++ b/lib/src/segy.c
@@ -549,15 +549,18 @@ int segy_field_forall( segy_file* fp,
     /*
      * non-mmap path. Doing multiple freads is slow, so instead the *actual*
      * offset is computed, not just the start of the header, and that's copied
-     * into the correct offset in our local buffer.
+     * into the correct offset in our local buffer. Note that byte offsets are
+     * exposed 1-indexed (to stay consistent with the specification), but the
+     * buffers are 0-indexed.
      *
      * Always read 4 bytes to be sure, there's no significant cost difference.
      */
     size_t readc;
+    const int zfield = field - 1;
     for( int i = start; slicelen > 0; i += step, ++buf, --slicelen ) {
-        err = segy_seek( fp, i, trace0 + field, trace_bsize );
+        err = segy_seek( fp, i, trace0 + zfield, trace_bsize );
         if( err != 0 ) return SEGY_FSEEK_ERROR;
-        readc = fread( header + field, sizeof( uint32_t ), 1, fp->fp );
+        readc = fread( header + zfield, sizeof( uint32_t ), 1, fp->fp );
         if( readc != 1 ) return SEGY_FREAD_ERROR;
 
         segy_get_field( header, field, &f );
diff --git a/python/test/segy.py b/python/test/segy.py
index 643ab2b..c3a52bf 100644
--- a/python/test/segy.py
+++ b/python/test/segy.py
@@ -260,6 +260,10 @@ class TestSegy(TestCase):
             attrxls = list(map(int, f.attributes(xl)[::-1]))
             self.assertListEqual(xls, attrxls)
 
+            self.assertEqual(f.header[0][il], f.attributes(il)[0])
+            f.mmap()
+            self.assertEqual(f.header[0][il], f.attributes(il)[0])
+
             ils = [(i // 5) + 1 for i in range(25)][1:21:3]
             attrils = list(map(int, f.attributes(il)[1:21:3]))
             self.assertListEqual(ils, attrils)

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/debian-science/packages/segyio.git