Hi Daniel, Thanks for taking a look. However ...
On Sun, Jan 30, 2011 at 4:17 AM, <danie...@apache.org> wrote: > Author: danielsh > Date: Sun Jan 30 03:17:54 2011 > New Revision: 1065170 > > URL: http://svn.apache.org/viewvc?rev=1065170&view=rev > Log: > On the 'diff-optimizations-bytes' branch: > > Silence compiler warnings. > > * subversion/libsvn_diff/lcs.c > (svn_diff__lcs): Add braces. > > * subversion/libsvn_diff/diff_file.c > (LOWER_7BITS_SET, BIT_7_SET, R_MASK, N_MASK, contains_eol): > Hide their definitions when they aren't used. > (find_identical_prefix): > Remove unused variables. > > Modified: > > subversion/branches/diff-optimizations-bytes/subversion/libsvn_diff/diff_file.c > subversion/branches/diff-optimizations-bytes/subversion/libsvn_diff/lcs.c > > Modified: > subversion/branches/diff-optimizations-bytes/subversion/libsvn_diff/diff_file.c > URL: > http://svn.apache.org/viewvc/subversion/branches/diff-optimizations-bytes/subversion/libsvn_diff/diff_file.c?rev=1065170&r1=1065169&r2=1065170&view=diff > ============================================================================== > --- > subversion/branches/diff-optimizations-bytes/subversion/libsvn_diff/diff_file.c > (original) > +++ > subversion/branches/diff-optimizations-bytes/subversion/libsvn_diff/diff_file.c > Sun Jan 30 03:17:54 2011 > @@ -338,6 +338,7 @@ is_one_at_eof(struct file_info file[], a > /* Quickly determine whether there is a eol char in CHUNK. > * (mainly copy-n-paste from eol.c#svn_eol__find_eol_start). > */ > +#if SVN_UNALIGNED_ACCESS_IS_OK > #if APR_SIZEOF_VOIDP == 8 > # define LOWER_7BITS_SET 0x7f7f7f7f7f7f7f7f > # define BIT_7_SET 0x8080808080808080 > @@ -349,7 +350,9 @@ is_one_at_eof(struct file_info file[], a > # define R_MASK 0x0a0a0a0a > # define N_MASK 0x0d0d0d0d > #endif > +#endif > > +#if SVN_UNALIGNED_ACCESS_IS_OK > static svn_boolean_t contains_eol(apr_size_t chunk) > { > apr_size_t r_test = chunk ^ R_MASK; > @@ -360,6 +363,7 @@ static svn_boolean_t contains_eol(apr_si > > return (r_test & n_test & BIT_7_SET) != BIT_7_SET; > } > +#endif > > /* Find the prefix which is identical between all elements of the FILE array. > * Return the number of prefix lines in PREFIX_LINES. REACHED_ONE_EOF will be > @@ -377,7 +381,6 @@ find_identical_prefix(svn_boolean_t *rea > svn_boolean_t had_cr = FALSE; > svn_boolean_t is_match; > apr_off_t lines = 0; > - apr_ssize_t max_delta, delta; ... hold on a minute :-). These variable are used, although only inside the SVN_UNALIGNED_ACCESS_IS_OK block. They are used around line 416 and following. > apr_size_t i; > > for (i = 1, is_match = TRUE; i < file_len; i++) > @@ -503,7 +506,7 @@ find_identical_suffix(struct file_info f > apr_off_t suffix_min_offset0; > apr_off_t min_file_size; > int suffix_lines_to_keep = SUFFIX_LINES_TO_KEEP; > - svn_boolean_t is_match, can_read, can_read_word, reached_prefix; > + svn_boolean_t is_match, can_read, reached_prefix; Same here. The variable 'can_read_word' is used inside the SVN_UNALIGNED_ACCESS_IS_OK block, at line 573 and following. So, how should these be handled? Can we only declare those two variables if SVN_UNALIGNED_ACCESS_IS_OK? Or maybe we can rearrange the code a little bit to make it more clear at the same time ... > apr_size_t i; > > memset(file_for_suffix, 0, sizeof(file_for_suffix)); > > Modified: > subversion/branches/diff-optimizations-bytes/subversion/libsvn_diff/lcs.c > URL: > http://svn.apache.org/viewvc/subversion/branches/diff-optimizations-bytes/subversion/libsvn_diff/lcs.c?rev=1065170&r1=1065169&r2=1065170&view=diff > ============================================================================== > --- subversion/branches/diff-optimizations-bytes/subversion/libsvn_diff/lcs.c > (original) > +++ subversion/branches/diff-optimizations-bytes/subversion/libsvn_diff/lcs.c > Sun Jan 30 03:17:54 2011 > @@ -217,10 +217,12 @@ svn_diff__lcs(svn_diff__position_t *posi > lcs->next = NULL; > > if (position_list1 == NULL || position_list2 == NULL) > - if (prefix_lines) > - return prepend_prefix_lcs(lcs, prefix_lines, pool); > - else > - return lcs; > + { > + if (prefix_lines) > + return prepend_prefix_lcs(lcs, prefix_lines, pool); > + else > + return lcs; > + } Yes, that's much better :-). Thanks, -- Johan
