You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2013/01/09 18:54:41 UTC

svn commit: r1430975 - /subversion/branches/fsfs-format7/subversion/libsvn_fs_fs/low_level.c

Author: stefan2
Date: Wed Jan  9 17:54:40 2013
New Revision: 1430975

URL: http://svn.apache.org/viewvc?rev=1430975&view=rev
Log:
On the fsfs-format7 branch: Unbreak the tests.

* subversion/libsvn_fs_fs/low_level.c
  (svn_fs_fs__write_rep_header): fix test for PLAIN reps

Modified:
    subversion/branches/fsfs-format7/subversion/libsvn_fs_fs/low_level.c

Modified: subversion/branches/fsfs-format7/subversion/libsvn_fs_fs/low_level.c
URL: http://svn.apache.org/viewvc/subversion/branches/fsfs-format7/subversion/libsvn_fs_fs/low_level.c?rev=1430975&r1=1430974&r2=1430975&view=diff
==============================================================================
--- subversion/branches/fsfs-format7/subversion/libsvn_fs_fs/low_level.c (original)
+++ subversion/branches/fsfs-format7/subversion/libsvn_fs_fs/low_level.c Wed Jan  9 17:54:40 2013
@@ -689,7 +689,7 @@ svn_fs_fs__write_rep_header(svn_fs_fs__r
 {
   const char *text;
   
-  if (header->is_delta)
+  if (header->is_delta == FALSE)
     {
       text = REP_PLAIN "\n";
     }



Re: svn commit: r1430975 - /subversion/branches/fsfs-format7/subversion/libsvn_fs_fs/low_level.c

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Wed, Jan 9, 2013 at 8:11 PM, Branko Čibej <br...@wandisco.com> wrote:

> On 09.01.2013 20:00, Stefan Fuhrmann wrote:
> > On Wed, Jan 9, 2013 at 7:42 PM, Branko Čibej <brane@wandisco.com
> > <ma...@wandisco.com>> wrote:
> >
> >     On 09.01.2013 19:35, Ben Reser wrote:
> >     > On Wed, Jan 9, 2013 at 10:00 AM, Branko Čibej
> >     <brane@wandisco.com <ma...@wandisco.com>> wrote:
> >     >> On 09.01.2013 18:54, stefan2@apache.org
> >     <ma...@apache.org> wrote:
> >     >>> -  if (header->is_delta)
> >     >>> +  if (header->is_delta == FALSE)
> >     >> Can we please use logical operators to test boolean values, not
> >     >> arithmetic ones?
> >
> >
> > Hm. I somehow got the impression from other people's
> > code that they liked the more explicit version.
>
> Yah, I noticed some people prefer to code COBOL in C. :)
>
> > Personally, I prefer the shorter one.
>
> Agreed. Of course I expect this to escalate into a flamewar, but I'll
> note that while "== FALSE" is marginally acceptable, "== TRUE" is not,
> because of C's rules about truth values.
>

r1431017 has the flame bait.

-- Stefan^2.

-- 
Certified & Supported Apache Subversion Downloads:
*

http://www.wandisco.com/subversion/download
*

Re: svn commit: r1430975 - /subversion/branches/fsfs-format7/subversion/libsvn_fs_fs/low_level.c

Posted by Branko Čibej <br...@wandisco.com>.
On 09.01.2013 20:00, Stefan Fuhrmann wrote:
> On Wed, Jan 9, 2013 at 7:42 PM, Branko Čibej <brane@wandisco.com
> <ma...@wandisco.com>> wrote:
>
>     On 09.01.2013 19:35, Ben Reser wrote:
>     > On Wed, Jan 9, 2013 at 10:00 AM, Branko Čibej
>     <brane@wandisco.com <ma...@wandisco.com>> wrote:
>     >> On 09.01.2013 18:54, stefan2@apache.org
>     <ma...@apache.org> wrote:
>     >>> -  if (header->is_delta)
>     >>> +  if (header->is_delta == FALSE)
>     >> Can we please use logical operators to test boolean values, not
>     >> arithmetic ones?
>
>
> Hm. I somehow got the impression from other people's
> code that they liked the more explicit version.

Yah, I noticed some people prefer to code COBOL in C. :)

> Personally, I prefer the shorter one.

Agreed. Of course I expect this to escalate into a flamewar, but I'll
note that while "== FALSE" is marginally acceptable, "== TRUE" is not,
because of C's rules about truth values.

-- Brane


-- 
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com


Re: svn commit: r1430975 - /subversion/branches/fsfs-format7/subversion/libsvn_fs_fs/low_level.c

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Wed, Jan 9, 2013 at 7:42 PM, Branko Čibej <br...@wandisco.com> wrote:

> On 09.01.2013 19:35, Ben Reser wrote:
> > On Wed, Jan 9, 2013 at 10:00 AM, Branko Čibej <br...@wandisco.com>
> wrote:
> >> On 09.01.2013 18:54, stefan2@apache.org wrote:
> >>> -  if (header->is_delta)
> >>> +  if (header->is_delta == FALSE)
> >> Can we please use logical operators to test boolean values, not
> >> arithmetic ones?
>

Hm. I somehow got the impression from other people's
code that they liked the more explicit version. Personally,
I prefer the shorter one.


> >> if (!header->is_delta)
> > There's quite a few examples of this in the code right now that we
> > should fix then
>
> Agreed -- we should fix them.
>

Ok. I'm on it right now. Being bored at the airport anyways ...

-- Stefan^2.

-- 
Certified & Supported Apache Subversion Downloads:
*

http://www.wandisco.com/subversion/download
*

Re: svn commit: r1430975 - /subversion/branches/fsfs-format7/subversion/libsvn_fs_fs/low_level.c

Posted by Branko Čibej <br...@wandisco.com>.
On 09.01.2013 19:35, Ben Reser wrote:
> On Wed, Jan 9, 2013 at 10:00 AM, Branko Čibej <br...@wandisco.com> wrote:
>> On 09.01.2013 18:54, stefan2@apache.org wrote:
>>> -  if (header->is_delta)
>>> +  if (header->is_delta == FALSE)
>> Can we please use logical operators to test boolean values, not
>> arithmetic ones?
>>
>> if (!header->is_delta)
> There's quite a few examples of this in the code right now that we
> should fix then

Agreed -- we should fix them.

-- Brane


-- 
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com


Re: svn commit: r1430975 - /subversion/branches/fsfs-format7/subversion/libsvn_fs_fs/low_level.c

Posted by Ben Reser <be...@reser.org>.
On Wed, Jan 9, 2013 at 10:00 AM, Branko Čibej <br...@wandisco.com> wrote:
> On 09.01.2013 18:54, stefan2@apache.org wrote:
>>
>> -  if (header->is_delta)
>> +  if (header->is_delta == FALSE)
>
> Can we please use logical operators to test boolean values, not
> arithmetic ones?
>
> if (!header->is_delta)

There's quite a few examples of this in the code right now that we
should fix then

[[[
subversion/svn/svn.c:  if (descend == FALSE)
subversion/libsvn_subr/win32_crashrpt.c:  if (log_params == FALSE &&
sym_info->Flags & SYMFLAG_LOCAL)
subversion/libsvn_subr/win32_crashrpt.c:  if
(get_temp_filename(dmp_filename, LOGFILE_PREFIX, "dmp") == FALSE ||
subversion/libsvn_subr/win32_crashrpt.c:
get_temp_filename(log_filename, LOGFILE_PREFIX, "log") == FALSE)
subversion/libsvn_subr/win32_crashrpt.c:  if (load_dbghelp_dll() == FALSE)
subversion/libsvn_subr/deprecated.c:              if (have_options == FALSE)
subversion/libsvn_subr/subst.c:              if (keyword_matches == FALSE)
subversion/libsvn_subr/subst.c:              if (keyword_matches == FALSE ||
subversion/libsvn_subr/cmdline.c:  if (non_interactive == FALSE)
subversion/libsvn_subr/cmdline.c:  if (non_interactive == FALSE)
subversion/libsvn_subr/cmdline.c:  if (non_interactive == FALSE)
subversion/libsvn_subr/opt.c:              if (have_options == FALSE)
subversion/libsvn_ra_svn/client.c:  if (v == FALSE)
subversion/libsvn_repos/authz.c:
section_name) == FALSE
subversion/libsvn_repos/authz.c:
section_name) == FALSE)
subversion/libsvn_repos/fs-wrap.c:          if
(svn_utf__is_valid(value->data, value->len) == FALSE)
subversion/libsvn_repos/authz.c~:
section_name) == FALSE
subversion/libsvn_repos/authz.c~:
section_name) == FALSE)
subversion/libsvn_fs_fs/fs_fs.c~:  ((may_be_corrupt == FALSE ||
(checksum) != NULL)     \
subversion/libsvn_fs_fs/fs_fs.c~:  if (ra->is_delta == FALSE)
subversion/libsvn_fs_fs/fs_fs.c~:      if (rep_args->is_delta == FALSE)
subversion/libsvn_fs_fs/fs_fs.c:  ((may_be_corrupt == FALSE ||
(checksum) != NULL)     \
subversion/libsvn_fs_fs/fs_fs.c:  if (ra->is_delta == FALSE)
subversion/libsvn_fs_fs/fs_fs.c:      if (rep_args->is_delta == FALSE)
subversion/libsvn_fs_base/fs.c:          if (files_match == FALSE)
subversion/svnsync/svnsync.c:      if (!filter || filter(propname) == FALSE)
subversion/libsvn_ra_serf/property.c:  if (requested_allprop == FALSE)
subversion/libsvn_ra_serf/property.c:  if (requested_allprop == FALSE)
subversion/libsvn_ra_serf/locks.c:  if (ctx->read_headers == FALSE)
subversion/libsvn_ra_serf/util.c:  if (sl.code == 404 &&
ctx->ignore_errors == FALSE)
subversion/libsvn_ra_serf/update.c:          if
(fetch_ctx->aborted_read == FALSE && fetch_ctx->read_size)
subversion/libsvn_ra_serf/update.c:  if (fetch_ctx->read_headers == FALSE)
subversion/libsvn_ra_serf/update.c:      if (info->lock_token &&
info->fetch_props == FALSE)
subversion/libsvn_ra_serf/update.c:      if (report->closed_root ==
FALSE && report->root_dir != NULL)
subversion/tests/libsvn_subr/subst_translate-test.c:
SVN_TEST_ASSERT(translated_line_endings == FALSE);
subversion/tests/libsvn_subr/stream-test.c:  SVN_TEST_ASSERT(read_only
== FALSE);
subversion/tests/libsvn_subr/stream-test.c:  SVN_TEST_ASSERT(read_only
== FALSE);
subversion/tests/libsvn_wc/db-test.c:  SVN_TEST_ASSERT(had_props == FALSE);
subversion/tests/libsvn_wc/db-test.c:  SVN_TEST_ASSERT(had_props == FALSE);
subversion/tests/libsvn_wc/db-test.c:  SVN_TEST_ASSERT(props_mod == FALSE);
subversion/tests/libsvn_wc/db-test.c:  SVN_TEST_ASSERT(conflicted == FALSE);
subversion/libsvn_delta/svndiff.c:  if (eb->header_done == FALSE)
subversion/libsvn_delta/svndiff.c:  if (eb->header_done == FALSE)
tools/client-side/svn-bench/svn-bench.c:  if (descend == FALSE)
tools/client-side/svn-bench/main.c~:  if (descend == FALSE)
]]]

[[[
subversion/libsvn_ra_svn/client.c:  if (v == TRUE)
subversion/tests/libsvn_subr/subst_translate-test.c:
SVN_TEST_ASSERT(translated_to_utf8 == TRUE);
subversion/tests/libsvn_subr/string-test.c:  if
(svn_stringbuf_compare(a, b) == TRUE)
subversion/tests/libsvn_wc/db-test.c:  SVN_TEST_ASSERT(have_base == TRUE);
subversion/tests/libsvn_wc/db-test.c:  SVN_TEST_ASSERT(have_work == TRUE);
]]]

Re: svn commit: r1430975 - /subversion/branches/fsfs-format7/subversion/libsvn_fs_fs/low_level.c

Posted by Ben Reser <be...@reser.org>.
On Wed, Jan 9, 2013 at 10:00 AM, Branko Čibej <br...@wandisco.com> wrote:
> On 09.01.2013 18:54, stefan2@apache.org wrote:
>>
>> -  if (header->is_delta)
>> +  if (header->is_delta == FALSE)
>
> Can we please use logical operators to test boolean values, not
> arithmetic ones?
>
> if (!header->is_delta)

There's quite a few examples of this in the code right now that we
should fix then

[[[
subversion/svn/svn.c:  if (descend == FALSE)
subversion/libsvn_subr/win32_crashrpt.c:  if (log_params == FALSE &&
sym_info->Flags & SYMFLAG_LOCAL)
subversion/libsvn_subr/win32_crashrpt.c:  if
(get_temp_filename(dmp_filename, LOGFILE_PREFIX, "dmp") == FALSE ||
subversion/libsvn_subr/win32_crashrpt.c:
get_temp_filename(log_filename, LOGFILE_PREFIX, "log") == FALSE)
subversion/libsvn_subr/win32_crashrpt.c:  if (load_dbghelp_dll() == FALSE)
subversion/libsvn_subr/deprecated.c:              if (have_options == FALSE)
subversion/libsvn_subr/subst.c:              if (keyword_matches == FALSE)
subversion/libsvn_subr/subst.c:              if (keyword_matches == FALSE ||
subversion/libsvn_subr/cmdline.c:  if (non_interactive == FALSE)
subversion/libsvn_subr/cmdline.c:  if (non_interactive == FALSE)
subversion/libsvn_subr/cmdline.c:  if (non_interactive == FALSE)
subversion/libsvn_subr/opt.c:              if (have_options == FALSE)
subversion/libsvn_ra_svn/client.c:  if (v == FALSE)
subversion/libsvn_repos/authz.c:
section_name) == FALSE
subversion/libsvn_repos/authz.c:
section_name) == FALSE)
subversion/libsvn_repos/fs-wrap.c:          if
(svn_utf__is_valid(value->data, value->len) == FALSE)
subversion/libsvn_repos/authz.c~:
section_name) == FALSE
subversion/libsvn_repos/authz.c~:
section_name) == FALSE)
subversion/libsvn_fs_fs/fs_fs.c~:  ((may_be_corrupt == FALSE ||
(checksum) != NULL)     \
subversion/libsvn_fs_fs/fs_fs.c~:  if (ra->is_delta == FALSE)
subversion/libsvn_fs_fs/fs_fs.c~:      if (rep_args->is_delta == FALSE)
subversion/libsvn_fs_fs/fs_fs.c:  ((may_be_corrupt == FALSE ||
(checksum) != NULL)     \
subversion/libsvn_fs_fs/fs_fs.c:  if (ra->is_delta == FALSE)
subversion/libsvn_fs_fs/fs_fs.c:      if (rep_args->is_delta == FALSE)
subversion/libsvn_fs_base/fs.c:          if (files_match == FALSE)
subversion/svnsync/svnsync.c:      if (!filter || filter(propname) == FALSE)
subversion/libsvn_ra_serf/property.c:  if (requested_allprop == FALSE)
subversion/libsvn_ra_serf/property.c:  if (requested_allprop == FALSE)
subversion/libsvn_ra_serf/locks.c:  if (ctx->read_headers == FALSE)
subversion/libsvn_ra_serf/util.c:  if (sl.code == 404 &&
ctx->ignore_errors == FALSE)
subversion/libsvn_ra_serf/update.c:          if
(fetch_ctx->aborted_read == FALSE && fetch_ctx->read_size)
subversion/libsvn_ra_serf/update.c:  if (fetch_ctx->read_headers == FALSE)
subversion/libsvn_ra_serf/update.c:      if (info->lock_token &&
info->fetch_props == FALSE)
subversion/libsvn_ra_serf/update.c:      if (report->closed_root ==
FALSE && report->root_dir != NULL)
subversion/tests/libsvn_subr/subst_translate-test.c:
SVN_TEST_ASSERT(translated_line_endings == FALSE);
subversion/tests/libsvn_subr/stream-test.c:  SVN_TEST_ASSERT(read_only
== FALSE);
subversion/tests/libsvn_subr/stream-test.c:  SVN_TEST_ASSERT(read_only
== FALSE);
subversion/tests/libsvn_wc/db-test.c:  SVN_TEST_ASSERT(had_props == FALSE);
subversion/tests/libsvn_wc/db-test.c:  SVN_TEST_ASSERT(had_props == FALSE);
subversion/tests/libsvn_wc/db-test.c:  SVN_TEST_ASSERT(props_mod == FALSE);
subversion/tests/libsvn_wc/db-test.c:  SVN_TEST_ASSERT(conflicted == FALSE);
subversion/libsvn_delta/svndiff.c:  if (eb->header_done == FALSE)
subversion/libsvn_delta/svndiff.c:  if (eb->header_done == FALSE)
tools/client-side/svn-bench/svn-bench.c:  if (descend == FALSE)
tools/client-side/svn-bench/main.c~:  if (descend == FALSE)
]]]

[[[
subversion/libsvn_ra_svn/client.c:  if (v == TRUE)
subversion/tests/libsvn_subr/subst_translate-test.c:
SVN_TEST_ASSERT(translated_to_utf8 == TRUE);
subversion/tests/libsvn_subr/string-test.c:  if
(svn_stringbuf_compare(a, b) == TRUE)
subversion/tests/libsvn_wc/db-test.c:  SVN_TEST_ASSERT(have_base == TRUE);
subversion/tests/libsvn_wc/db-test.c:  SVN_TEST_ASSERT(have_work == TRUE);
]]]

Re: svn commit: r1430975 - /subversion/branches/fsfs-format7/subversion/libsvn_fs_fs/low_level.c

Posted by Branko Čibej <br...@wandisco.com>.
On 09.01.2013 18:54, stefan2@apache.org wrote:
>  
> -  if (header->is_delta)
> +  if (header->is_delta == FALSE)

Can we please use logical operators to test boolean values, not
arithmetic ones?

if (!header->is_delta)


-- Brane

-- 
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com