You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Greg Hudson <gh...@MIT.EDU> on 2004/04/29 16:06:39 UTC
Sign error in apr_file_seek
APR's handling of seeks to APR_END in buffered files has a sign error.
I've included a patch (see the end), and a test case.
I verified that nothing in Subversion or httpd uses APR_END, so it
seems likely that nothing is relying on the sign error. And the
documentation of apr_file_seek is very clear:
* APR_END -- add the offset to the current file size
I have tested the fix on Unix. I can't test the change to the Windows
or OS/2 code, but it's a pretty obvious fix.
---- Test case ---
#include <apr.h>
#include <apr_pools.h>
#include <apr_file_io.h>
int main()
{
apr_pool_t *pool;
apr_file_t *file;
char buf[128];
apr_size_t len;
apr_off_t off;
apr_initialize();
apr_pool_create(&pool, NULL);
apr_file_open(&file, "testfile", APR_READ|APR_BUFFERED, 0, pool);
off = -5;
apr_file_seek(file, APR_END, &off);
len = sizeof(buf) - 1;
apr_file_read(file, buf, &len);
buf[len] = 0;
printf("%s", buf);
return 0;
}
To see the problem, echo something like "1234567890" into the file
"testfile" and then compile and run the test case. You will see no
output. If you remove the APR_BUFFERED flag, you will see "7890\n" as
expected.
Index: os2/seek.c
===================================================================
RCS file: /home/cvspublic/apr/file_io/os2/seek.c,v
retrieving revision 1.26
diff -u -r1.26 seek.c
--- os2/seek.c 13 Feb 2004 09:38:24 -0000 1.26
+++ os2/seek.c 29 Apr 2004 14:01:40 -0000
@@ -70,7 +70,7 @@
case APR_END:
rc = apr_file_info_get(&finfo, APR_FINFO_NORM, thefile);
if (rc == APR_SUCCESS)
- rc = setptr(thefile, finfo.size - *offset);
+ rc = setptr(thefile, finfo.size + *offset);
break;
}
Index: unix/seek.c
===================================================================
RCS file: /home/cvspublic/apr/file_io/unix/seek.c,v
retrieving revision 1.34
diff -u -r1.34 seek.c
--- unix/seek.c 27 Mar 2004 13:11:17 -0000 1.34
+++ unix/seek.c 29 Apr 2004 14:01:40 -0000
@@ -69,7 +69,7 @@
case APR_END:
rc = apr_file_info_get(&finfo, APR_FINFO_SIZE, thefile);
if (rc == APR_SUCCESS)
- rc = setptr(thefile, finfo.size - *offset);
+ rc = setptr(thefile, finfo.size + *offset);
break;
}
Index: win32/seek.c
===================================================================
RCS file: /home/cvspublic/apr/file_io/win32/seek.c,v
retrieving revision 1.28
diff -u -r1.28 seek.c
--- win32/seek.c 13 Feb 2004 09:38:27 -0000 1.28
+++ win32/seek.c 29 Apr 2004 14:01:40 -0000
@@ -73,7 +73,7 @@
case APR_END:
rc = apr_file_info_get(&finfo, APR_FINFO_SIZE, thefile);
if (rc == APR_SUCCESS)
- rc = setptr(thefile, finfo.size - *offset);
+ rc = setptr(thefile, finfo.size + *offset);
break;
default:
Re: Sign error in apr_file_seek
Posted by Jeff Trawick <tr...@attglobal.net>.
Greg Hudson wrote:
> On Fri, 2004-04-30 at 18:58, Joe Orton wrote:
>
>>On Thu, Apr 29, 2004 at 10:44:16AM -0400, Jeff Trawick wrote:
>>
>>>wow
>>>
>>>is there any chance you can merge your test case into the existing test
>>>suite?
>
>
> Index: file_io/os2/seek.c
> Index: file_io/unix/seek.c
> Index: file_io/win32/seek.c
> Index: test/testfile.c
thanks a bunch!
fix committed to 1.0-dev, but I should commit to 0.9 branch as well
Re: Sign error in apr_file_seek
Posted by Greg Hudson <gh...@MIT.EDU>.
On Fri, 2004-04-30 at 18:58, Joe Orton wrote:
> On Thu, Apr 29, 2004 at 10:44:16AM -0400, Jeff Trawick wrote:
> > wow
> >
> > is there any chance you can merge your test case into the existing test
> > suite?
Index: file_io/os2/seek.c
===================================================================
RCS file: /home/cvspublic/apr/file_io/os2/seek.c,v
retrieving revision 1.26
diff -u -r1.26 seek.c
--- file_io/os2/seek.c 13 Feb 2004 09:38:24 -0000 1.26
+++ file_io/os2/seek.c 1 May 2004 18:38:40 -0000
@@ -70,7 +70,7 @@
case APR_END:
rc = apr_file_info_get(&finfo, APR_FINFO_NORM, thefile);
if (rc == APR_SUCCESS)
- rc = setptr(thefile, finfo.size - *offset);
+ rc = setptr(thefile, finfo.size + *offset);
break;
}
Index: file_io/unix/seek.c
===================================================================
RCS file: /home/cvspublic/apr/file_io/unix/seek.c,v
retrieving revision 1.34
diff -u -r1.34 seek.c
--- file_io/unix/seek.c 27 Mar 2004 13:11:17 -0000 1.34
+++ file_io/unix/seek.c 1 May 2004 18:38:40 -0000
@@ -69,7 +69,7 @@
case APR_END:
rc = apr_file_info_get(&finfo, APR_FINFO_SIZE, thefile);
if (rc == APR_SUCCESS)
- rc = setptr(thefile, finfo.size - *offset);
+ rc = setptr(thefile, finfo.size + *offset);
break;
}
Index: file_io/win32/seek.c
===================================================================
RCS file: /home/cvspublic/apr/file_io/win32/seek.c,v
retrieving revision 1.28
diff -u -r1.28 seek.c
--- file_io/win32/seek.c 13 Feb 2004 09:38:27 -0000 1.28
+++ file_io/win32/seek.c 1 May 2004 18:38:40 -0000
@@ -73,7 +73,7 @@
case APR_END:
rc = apr_file_info_get(&finfo, APR_FINFO_SIZE, thefile);
if (rc == APR_SUCCESS)
- rc = setptr(thefile, finfo.size - *offset);
+ rc = setptr(thefile, finfo.size + *offset);
break;
default:
Index: test/testfile.c
===================================================================
RCS file: /home/cvspublic/apr/test/testfile.c,v
retrieving revision 1.72
diff -u -r1.72 testfile.c
--- test/testfile.c 13 Feb 2004 09:38:34 -0000 1.72
+++ test/testfile.c 1 May 2004 18:38:40 -0000
@@ -228,6 +228,27 @@
CuAssertStrEquals(tc, TESTSTR + 5, str);
apr_file_close(filetest);
+
+ /* Test for regression of sign error bug with SEEK_END and
+ buffered files. */
+ rv = apr_file_open(&filetest, FILENAME,
+ APR_READ | APR_BUFFERED,
+ APR_UREAD | APR_UWRITE | APR_GREAD, p);
+ apr_assert_success(tc, "Open test file " FILENAME, rv);
+
+ offset = -5;
+ rv = apr_file_seek(filetest, SEEK_END, &offset);
+ CuAssertIntEquals(tc, APR_SUCCESS, rv);
+ CuAssertIntEquals(tc, strlen(TESTSTR) - 5, nbytes);
+
+ memset(str, 0, nbytes + 1);
+ nbytes = 256;
+ rv = apr_file_read(filetest, str, &nbytes);
+ CuAssertIntEquals(tc, APR_SUCCESS, rv);
+ CuAssertIntEquals(tc, 5, nbytes);
+ CuAssertStrEquals(tc, TESTSTR + strlen(TESTSTR) - 5, str);
+
+ apr_file_close(filetest);
}
static void test_userdata_set(CuTest *tc)
Re: Sign error in apr_file_seek
Posted by Joe Orton <jo...@manyfish.co.uk>.
Greg wasn't subscribed to dev@apr before...
On Thu, Apr 29, 2004 at 10:44:16AM -0400, Jeff Trawick wrote:
> Greg Hudson wrote:
> >APR's handling of seeks to APR_END in buffered files has a sign error.
> >I've included a patch (see the end), and a test case.
>
> wow
>
> is there any chance you can merge your test case into the existing test
> suite?
Re: Sign error in apr_file_seek
Posted by Jeff Trawick <tr...@attglobal.net>.
Greg Hudson wrote:
> APR's handling of seeks to APR_END in buffered files has a sign error.
> I've included a patch (see the end), and a test case.
wow
is there any chance you can merge your test case into the existing test suite?