Ben Reser wrote:
> Patch is up in Issue #1705. It's also marked 1.0-cand and in the
> STATUS file.
[from the patch]
>+ /* This is probably overly paranoid about checking
>+ * but I'd rather be safe than sorry */
>+ if ((sizeof (apr_off_t) == 4 &&
>+ (*offset < -0x7FFFFFFF || *offset > 0x7FFFFFFF)) ||
>+ (sizeof (apr_off_t) == 2 &&
>+ (*offset < -0x7FFF || *offset > 0x7FFF)) ||
>+ (sizeof (apr_off_t) == 1 &&
>+ (*offset < -0x7F || *offset > 0x7FF)))
>+ {
>+ return svn_error_create (SVN_ERR_IO_OFFSET_SIZE, NULL,
>+ "Cannot seek file");
>+ }
If you really wanted to be paranoid you could write it like
if ((sizeof (apr_off_t) != 4 ||
*offset < -0x7FFFFFFF || *offset > 0x7FFFFFFF) &&
(sizeof (apr_off_t) != 2 ||
*offset < -0x7FFF || *offset > 0x7FFF) &&
(sizeof (apr_off_t) != 1 ||
*offset < -0x7F || *offset > 0x7FF))
{
return svn_error_create (SVN_ERR_IO_OFFSET_SIZE, NULL,
"Cannot seek file");
}
That way there'd be an error for 3, 5, 6, and 7 byte off_t's.
I'm also curious if another implementation might work too:
const apr_off_t min = ((apr_off_t)-1) << (sizeof(apr_off_t)*8-1);
const apr_off_t max = ~min;
if (*offset < min || *offset > max) ...
It assumes a 2's complement representation. Anyone know if that's ok in ansi
c?
[further down in patch]
>+ apr_offset = (apr_off_t) *offset;
>+
> return do_io_file_wrapper_cleanup
>- (file, apr_file_seek (file, where, offset),
>+ (file, apr_file_seek (file, where, &apr_offset),
> "set position pointer in", pool);
> }
The function declaration says "svn_offset_t *offset" instead of
"svn_offset_t const *offset." Does that mean the APR function might
manipulate the offset pointer? If so you'd need to forward the new value
back to the caller with:
*offset = apr_offset;
- Russ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jan 16 23:06:58 2004