On Wed, Jan 21, 2004 at 03:25:04PM -0000, bliss@tigris.org wrote:
> I'd really like this issue fixed, but the current patch has several bugs in
> diff_file.c. I have not checked the other files for similar errors. If you
> compile on a platform with 32-bit apr_off_t, gcc even tells you about it:
>
> diff_file.c: In function `read_chunk':
> diff_file.c:113: warning: passing arg 3 of `apr_file_seek' from incompatible
> pointer type
> diff_file.c: In function `svn_diff__file_output_unified_flush_hunk':
> diff_file.c:709: warning: long int format, different type arg (arg 4)
> diff_file.c:714: warning: long int format, different type arg (arg 4)
> diff_file.c:719: warning: long int format, different type arg (arg 4)
> diff_file.c:724: warning: long int format, different type arg (arg 4)
>
> All of these are serious errors. They are not hard too hard to fix, though.
Okay this took more work to fix than I thought initially. The error on
line 113 came from using apr_file_seek instead of svn_io_file_seek.
I've converted it using svn_io_file_seek there but read_chunk (and a
bunch of the libsvn_diff interface) didn't take a pool argument. This
lead to some changes needed to the API for libsvn_diff.
It was discussed on IRC this afternoon if we should remove the need for
the pool param to svn_io_file_seek (which we could do) or should add a
pool param to diff. The consensus of those who responded was to add
pool to diff.
On the diff issue I changed more of the interfaces than probably
necessary. If svn_diff_contains_conflicts and svn_diff_contains_diffs
or the token_discard and token_discard_all (part of svn_diff_fns_t)
needed to take pool parameters is questionable. I just did all of them
now. If we don't like all of them taking pool parameters it's probably
easier to remove some of the changes than it would be to go back and add
them to the patch. So I did this in the interest of expediency of
getting this issue closed.
The new patch for the issue is Patch 289 on Issue #1705.
You'll also want to apply the Patch 290 on Issue #1705 on top of that
patch. I left them split up to make it easier to review them.
This time I'm positive there are no warnings on 32-bit archs and it make
check's on a 32-bit arch and 64-bit arch.
Hopefully with this we can consider the issue done.
--
Ben Reser <ben@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jan 23 07:44:54 2004