You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Küng <to...@gmail.com> on 2009/04/16 19:30:27 UTC
boolean issue with serf
Hi,
A user reported a strange problem that happens only with TSVN and serf,
but not with the CL client.
http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=4061&dsMessageId=1752435
After some testing I found that TSVN passes a value of 32 as 'true' to
svn_client_lock for the --force flag. This works ok when using neon, but
fails when using serf. If I pass 1 instead of 32 then serf works too.
Now while I can fix this easily in TSVN, I'm wondering if it's really
required to pass 1 as svn_boolean_t values? Shouldn't all comparisons be
done as !=0 instead of == 1 in C since it doesn't have a bool type?
I haven't stepped through the serf code to find out where the comparison
is that fails if I pass 32 as 'TRUE' though...
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1753422
Re: boolean issue with serf
Posted by "Clark S. Cox III" <cl...@apple.com>.
On Apr 16, 2009, at 3:07 PM, Michae Sinz wrote:
> On Thu, Apr 16, 2009 at 1:36 PM, Greg Stein <gs...@gmail.com> wrote:
> On Thu, Apr 16, 2009 at 21:46, Stefan Küng <to...@gmail.com>
> wrote:
> > C. Michael Pilato wrote:
> [...]
> > Hmm - according to wikipedia:
> > http://en.wikipedia.org/wiki/Boolean_datatype
> >
> > "However, problems arise from the fact that any non-zero value
> > represents true in C, while the value TRUE is represented by a
> specific
> > value. So while in other languages, if (foo == TRUE) ... is merely
> > redundant, in C, it is actually incorrect code."
>
> svn_boolean_t is defined as one of {TRUE, FALSE}.
>
> Pass one of those values. Wikipedia does not define our svn_boolean_t.
>
> I agree that they don't define the svn_boolean_t but show me where
> the compiler will respect that or enforce that?
>
> Both at the user code path and in the actually evaluation of
> operations into a boolean.
>
> There are cases in code, depending on the compiler and optimizer,
> where in C code the compiler itself will not force the value to be 0
> or 1. In fact, a common optimization trick has been used to let
> whatever non-zero pass through or even use the non-zero to make -1
> (all bits on in whatever word size you expect). (Albeit these were
> highly optimizing embedded C compilers - it has been years since I
> have looked at C code generation and optimization paths and in other
> languages there is an actual boolean type that can only have two
> values and thus the behavior of the code is more strictly defined.)
>
> PS - on CPU architectures there are instructions that expand the CPU
> condition codes into a boolean value in a register such that no
> branching is needed at all to store the result of a boolean
> operation/test. In at least two of the cases these expand to -1 or
> 0 (all bits on or all bits off). I would expect that those
> platforms would end up having a natural boolean behavior of -1.
> This would make it impossible to correctly do:
>
> svn_boolean_t flag = (x > y);
>
> and expect flag to contain either TRUE or FALSE (it may contain
> another value).
This is not true. The C standard guarantees that the result of (x >
y), converted to any integer type (signed bitfields notwithstanding)
is always either exactly equal to 1 or exactly equal to 0. Any
compiler that fails to behave as such is broken.
>
> Thus the code would have to be written:
>
> svn_boolean_t flag = (x > y) ? TRUE : FALSE;
>
> which looks a bit redundant (and will be more code as the compiler
> will have to do a test-branch-store or test-failbranch-store-branch
> to store the correct value into flag or a register representing
> flag. With enough optimization smarts it may be able to figure out
> that you could do test-negate-store to get that to work out...
>
> --
> Michael Sinz Technology and Engineering Director/
> Consultant
> "Starting Startups" mailto:Michael.Sinz@sinz.org
> My place on the web http://www.sinz.org/Michael.Sinz
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1756735
Re: boolean issue with serf
Posted by Michae Sinz <Mi...@sinz.org>.
On Thu, Apr 16, 2009 at 1:36 PM, Greg Stein <gs...@gmail.com> wrote:
> On Thu, Apr 16, 2009 at 21:46, Stefan Küng <to...@gmail.com> wrote:
> > C. Michael Pilato wrote:
> [...]
> > Hmm - according to wikipedia:
> > http://en.wikipedia.org/wiki/Boolean_datatype
> >
> > "However, problems arise from the fact that any non-zero value
> > represents true in C, while the value TRUE is represented by a specific
> > value. So while in other languages, if (foo == TRUE) ... is merely
> > redundant, in C, it is actually incorrect code."
>
> svn_boolean_t is defined as one of {TRUE, FALSE}.
>
> Pass one of those values. Wikipedia does not define our svn_boolean_t.
I agree that they don't define the svn_boolean_t but show me where the
compiler will respect that or enforce that?
Both at the user code path and in the actually evaluation of operations into
a boolean.
There are cases in code, depending on the compiler and optimizer, where in C
code the compiler itself will not force the value to be 0 or 1. In fact, a
common optimization trick has been used to let whatever non-zero pass
through or even use the non-zero to make -1 (all bits on in whatever word
size you expect). (Albeit these were highly optimizing embedded C compilers
- it has been years since I have looked at C code generation and
optimization paths and in other languages there is an actual boolean type
that can only have two values and thus the behavior of the code is more
strictly defined.)
PS - on CPU architectures there are instructions that expand the CPU
condition codes into a boolean value in a register such that no branching is
needed at all to store the result of a boolean operation/test. In at least
two of the cases these expand to -1 or 0 (all bits on or all bits off). I
would expect that those platforms would end up having a natural boolean
behavior of -1. This would make it impossible to correctly do:
svn_boolean_t flag = (x > y);
and expect flag to contain either TRUE or FALSE (it may contain another
value).
Thus the code would have to be written:
svn_boolean_t flag = (x > y) ? TRUE : FALSE;
which looks a bit redundant (and will be more code as the compiler will have
to do a test-branch-store or test-failbranch-store-branch to store the
correct value into flag or a register representing flag. With enough
optimization smarts it may be able to figure out that you could do
test-negate-store to get that to work out...
--
Michael Sinz Technology and Engineering Director/Consultant
"Starting Startups" mailto:Michael.Sinz@sinz.org
My place on the web http://www.sinz.org/Michael.Sinz
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1755763
Re: boolean issue with serf
Posted by Greg Stein <gs...@gmail.com>.
On Thu, Apr 16, 2009 at 21:46, Stefan Küng <to...@gmail.com> wrote:
> C. Michael Pilato wrote:
>> Stefan Küng wrote:
>>> Hi,
>>>
>>> A user reported a strange problem that happens only with TSVN and serf,
>>> but not with the CL client.
>>> http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=4061&dsMessageId=1752435
>>>
>>> After some testing I found that TSVN passes a value of 32 as 'true' to
>>> svn_client_lock for the --force flag. This works ok when using neon, but
>>> fails when using serf. If I pass 1 instead of 32 then serf works too.
>>>
>>> Now while I can fix this easily in TSVN, I'm wondering if it's really
>>> required to pass 1 as svn_boolean_t values? Shouldn't all comparisons be
>>> done as !=0 instead of == 1 in C since it doesn't have a bool type?
>>
>> While I generally agree that for all intents and purposes, false == 0 and
>> true is anything else, we do provide the values TRUE and FALSE for use with
>> svn_boolean_t and it is not technically a contract violation to expect
>> exactly one of those two values to be provided by the caller.
>
> Hmm - according to wikipedia:
> http://en.wikipedia.org/wiki/Boolean_datatype
>
> "However, problems arise from the fact that any non-zero value
> represents true in C, while the value TRUE is represented by a specific
> value. So while in other languages, if (foo == TRUE) ... is merely
> redundant, in C, it is actually incorrect code."
svn_boolean_t is defined as one of {TRUE, FALSE}.
Pass one of those values. Wikipedia does not define our svn_boolean_t.
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1754509
Re: boolean issue with serf
Posted by Stefan Küng <to...@gmail.com>.
C. Michael Pilato wrote:
> Stefan Küng wrote:
>> Hi,
>>
>> A user reported a strange problem that happens only with TSVN and serf,
>> but not with the CL client.
>> http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=4061&dsMessageId=1752435
>>
>> After some testing I found that TSVN passes a value of 32 as 'true' to
>> svn_client_lock for the --force flag. This works ok when using neon, but
>> fails when using serf. If I pass 1 instead of 32 then serf works too.
>>
>> Now while I can fix this easily in TSVN, I'm wondering if it's really
>> required to pass 1 as svn_boolean_t values? Shouldn't all comparisons be
>> done as !=0 instead of == 1 in C since it doesn't have a bool type?
>
> While I generally agree that for all intents and purposes, false == 0 and
> true is anything else, we do provide the values TRUE and FALSE for use with
> svn_boolean_t and it is not technically a contract violation to expect
> exactly one of those two values to be provided by the caller.
>
Hmm - according to wikipedia:
http://en.wikipedia.org/wiki/Boolean_datatype
"However, problems arise from the fact that any non-zero value
represents true in C, while the value TRUE is represented by a specific
value. So while in other languages, if (foo == TRUE) ... is merely
redundant, in C, it is actually incorrect code."
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1753780
Re: boolean issue with serf
Posted by Stefan Küng <to...@gmail.com>.
C. Michael Pilato wrote:
> Stefan Küng wrote:
>> Hi,
>>
>> A user reported a strange problem that happens only with TSVN and serf,
>> but not with the CL client.
>> http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=4061&dsMessageId=1752435
>>
>> After some testing I found that TSVN passes a value of 32 as 'true' to
>> svn_client_lock for the --force flag. This works ok when using neon, but
>> fails when using serf. If I pass 1 instead of 32 then serf works too.
>>
>> Now while I can fix this easily in TSVN, I'm wondering if it's really
>> required to pass 1 as svn_boolean_t values? Shouldn't all comparisons be
>> done as !=0 instead of == 1 in C since it doesn't have a bool type?
>
> While I generally agree that for all intents and purposes, false == 0 and
> true is anything else, we do provide the values TRUE and FALSE for use with
> svn_boolean_t and it is not technically a contract violation to expect
> exactly one of those two values to be provided by the caller.
>
I guess I then have to adjust all calls to every svn function I call in
TSVN - Windows API's rarely return '1' for their BOOL value but only !=
0, and I often use Windows API's to determine some svn function params.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1753746
Re: boolean issue with serf
Posted by "C. Michael Pilato" <cm...@collab.net>.
Stefan Küng wrote:
> Hi,
>
> A user reported a strange problem that happens only with TSVN and serf,
> but not with the CL client.
> http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=4061&dsMessageId=1752435
>
> After some testing I found that TSVN passes a value of 32 as 'true' to
> svn_client_lock for the --force flag. This works ok when using neon, but
> fails when using serf. If I pass 1 instead of 32 then serf works too.
>
> Now while I can fix this easily in TSVN, I'm wondering if it's really
> required to pass 1 as svn_boolean_t values? Shouldn't all comparisons be
> done as !=0 instead of == 1 in C since it doesn't have a bool type?
While I generally agree that for all intents and purposes, false == 0 and
true is anything else, we do provide the values TRUE and FALSE for use with
svn_boolean_t and it is not technically a contract violation to expect
exactly one of those two values to be provided by the caller.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1753648
Re: boolean issue with serf
Posted by Stefan Küng <to...@gmail.com>.
Lieven Govaerts wrote:
> On 04/16/2009 09:39 PM, Stefan Küng wrote:
>> On Thu, Apr 16, 2009 at 21:30, Stefan Küng<to...@gmail.com> wrote:
> ..
>>
>> A quick search through the files revealed:
>>
>> libsvn_ra_serf\locks.c, line 720 (1.6.x branch), function
>> svn_ra_serf__unlock():
>> if (force == TRUE&& (!token || token[0] == '\0'))
>>
>> libsvn_ra_serf\locks.c, line 329(1.6.x branch), function
>> set_lock_headers():
>> if (lock_ctx->force == TRUE)
>>
>
> I would find it very counter-intuitive to change these to:
> if (lock_ctx->force != FALSE) ...
> ... and to ensure that people are going to remember a rule like this in
> the future.
Well, not
if (lock_ctx->force != FALSE)
but simply
if (lock_ctx->force)
And I think it's less intuitive to call all svn functions with
svn_function( myBOOLparam ? TRUE : FALSE );
instead of just
svn_function( myBOOLparam );
Also, I've learned very early that you should *never* compare a value to
TRUE or 1.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1753845
Re: boolean issue with serf
Posted by Greg Stein <gs...@gmail.com>.
On Sat, Apr 18, 2009 at 17:43, Stefan Küng <to...@gmail.com> wrote:
> Greg Stein wrote:
>> On Sat, Apr 18, 2009 at 14:07, Stefan Küng <to...@gmail.com> wrote:
>>> ...
>>> I've attached a patch which replaces all if(xxx == TRUE) statements with
>>> if(xxx) (well, all that I found in trunk with a simple grep search).
>>>
>>> Not sure about the commit message though: is it really necessary to list
>>> all functions that are affected for such a change?
>>
>> I'd say "nah, don't bother."
>
> With the log message or the commit?
I was responding to your question: I don't think it is really
necessary to list all the functions.
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1793354
Re: boolean issue with serf
Posted by Stefan Küng <to...@gmail.com>.
Greg Stein wrote:
> On Sat, Apr 18, 2009 at 14:07, Stefan Küng <to...@gmail.com> wrote:
>> ...
>> I've attached a patch which replaces all if(xxx == TRUE) statements with
>> if(xxx) (well, all that I found in trunk with a simple grep search).
>>
>> Not sure about the commit message though: is it really necessary to list
>> all functions that are affected for such a change?
>
> I'd say "nah, don't bother."
With the log message or the commit?
> I'd also recommend the log message uses svn-style for the code :-)
> (space after 'if' and no extra in-paren spaces)
Sure, will do.
> And lastly, I'd suggest you update your code to pass 0/1 rather than
> 0/nonzero for svn_boolean_t. Who knows what other code might be
> written "knowing" that an svn_boolean_t only has 0 or 1 values.
That I will definitely do.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1791856
Re: boolean issue with serf
Posted by David Glasser <gl...@davidglasser.net>.
2009/4/19 Branko Cibej <br...@xbc.nu>:
> Greg Stein wrote:
>> On Sat, Apr 18, 2009 at 14:07, Stefan Küng <to...@gmail.com> wrote:
>>
>>> ...
>>> I've attached a patch which replaces all if(xxx == TRUE) statements with
>>> if(xxx) (well, all that I found in trunk with a simple grep search).
>>>
>>> Not sure about the commit message though: is it really necessary to list
>>> all functions that are affected for such a change?
>>>
>>
>> I'd say "nah, don't bother."
>>
>> I'd also recommend the log message uses svn-style for the code :-)
>> (space after 'if' and no extra in-paren spaces)
>>
>> And lastly, I'd suggest you update your code to pass 0/1 rather than
>> 0/nonzero for svn_boolean_t. Who knows what other code might be
>> written "knowing" that an svn_boolean_t only has 0 or 1 values.
>>
>
> True ... I must say, though, that any such code is quite fundamentally
> broken. Regardless of our definition of svn_boolean_t and TRUE and FALSE
> (note that we don't even define those macrose if some external header
> already defines them for us!!). It's quite simply wrong to test "x ==
> TRUE" or "x == 1" in a boolean context in C. IMNSHO.
This seems like a reasonable place to apply Postel's Law.
--dave
--
glasser@davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1831647
Re: boolean issue with serf
Posted by Branko Cibej <br...@xbc.nu>.
Greg Stein wrote:
> On Sat, Apr 18, 2009 at 14:07, Stefan Küng <to...@gmail.com> wrote:
>
>> ...
>> I've attached a patch which replaces all if(xxx == TRUE) statements with
>> if(xxx) (well, all that I found in trunk with a simple grep search).
>>
>> Not sure about the commit message though: is it really necessary to list
>> all functions that are affected for such a change?
>>
>
> I'd say "nah, don't bother."
>
> I'd also recommend the log message uses svn-style for the code :-)
> (space after 'if' and no extra in-paren spaces)
>
> And lastly, I'd suggest you update your code to pass 0/1 rather than
> 0/nonzero for svn_boolean_t. Who knows what other code might be
> written "knowing" that an svn_boolean_t only has 0 or 1 values.
>
True ... I must say, though, that any such code is quite fundamentally
broken. Regardless of our definition of svn_boolean_t and TRUE and FALSE
(note that we don't even define those macrose if some external header
already defines them for us!!). It's quite simply wrong to test "x ==
TRUE" or "x == 1" in a boolean context in C. IMNSHO.
-- Brane
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1809395
Re: boolean issue with serf
Posted by Greg Stein <gs...@gmail.com>.
On Sat, Apr 18, 2009 at 14:07, Stefan Küng <to...@gmail.com> wrote:
>...
> I've attached a patch which replaces all if(xxx == TRUE) statements with
> if(xxx) (well, all that I found in trunk with a simple grep search).
>
> Not sure about the commit message though: is it really necessary to list
> all functions that are affected for such a change?
I'd say "nah, don't bother."
I'd also recommend the log message uses svn-style for the code :-)
(space after 'if' and no extra in-paren spaces)
And lastly, I'd suggest you update your code to pass 0/1 rather than
0/nonzero for svn_boolean_t. Who knows what other code might be
written "knowing" that an svn_boolean_t only has 0 or 1 values.
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1790591
Re: boolean issue with serf
Posted by Stefan Küng <to...@gmail.com>.
C. Michael Pilato wrote:
> Lieven Govaerts wrote:
>> On 04/16/2009 09:48 PM, Lieven Govaerts wrote:
>>> On 04/16/2009 09:39 PM, Stefan Küng wrote:
>>>> On Thu, Apr 16, 2009 at 21:30, Stefan Küng<to...@gmail.com>
>>>> wrote:
>>> ..
>>>> A quick search through the files revealed:
>>>>
>>>> libsvn_ra_serf\locks.c, line 720 (1.6.x branch), function
>>>> svn_ra_serf__unlock():
>>>> if (force == TRUE&& (!token || token[0] == '\0'))
>>>>
>>>> libsvn_ra_serf\locks.c, line 329(1.6.x branch), function
>>>> set_lock_headers():
>>>> if (lock_ctx->force == TRUE)
>>>>
>>> I would find it very counter-intuitive to change these to:
>>> if (lock_ctx->force != FALSE) ...
>>> ... and to ensure that people are going to remember a rule like this in
>>> the future.
>>>
>> ... most of our TRUE checks are done as "if (lock_ctx->force)" though,
>> wouldn't have a problem with that.
>
> Nor would I.
>
I've attached a patch which replaces all if(xxx == TRUE) statements with
if(xxx) (well, all that I found in trunk with a simple grep search).
Not sure about the commit message though: is it really necessary to list
all functions that are affected for such a change?
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1788451
Re: boolean issue with serf
Posted by "C. Michael Pilato" <cm...@collab.net>.
Lieven Govaerts wrote:
> On 04/16/2009 09:48 PM, Lieven Govaerts wrote:
>> On 04/16/2009 09:39 PM, Stefan Küng wrote:
>>> On Thu, Apr 16, 2009 at 21:30, Stefan Küng<to...@gmail.com>
>>> wrote:
>> ..
>>> A quick search through the files revealed:
>>>
>>> libsvn_ra_serf\locks.c, line 720 (1.6.x branch), function
>>> svn_ra_serf__unlock():
>>> if (force == TRUE&& (!token || token[0] == '\0'))
>>>
>>> libsvn_ra_serf\locks.c, line 329(1.6.x branch), function
>>> set_lock_headers():
>>> if (lock_ctx->force == TRUE)
>>>
>>
>> I would find it very counter-intuitive to change these to:
>> if (lock_ctx->force != FALSE) ...
>> ... and to ensure that people are going to remember a rule like this in
>> the future.
>>
> ... most of our TRUE checks are done as "if (lock_ctx->force)" though,
> wouldn't have a problem with that.
Nor would I.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1754313
Re: boolean issue with serf
Posted by Lieven Govaerts <sv...@mobsol.be>.
On 04/16/2009 09:48 PM, Lieven Govaerts wrote:
> On 04/16/2009 09:39 PM, Stefan Küng wrote:
>> On Thu, Apr 16, 2009 at 21:30, Stefan Küng<to...@gmail.com> wrote:
> ..
>> A quick search through the files revealed:
>>
>> libsvn_ra_serf\locks.c, line 720 (1.6.x branch), function svn_ra_serf__unlock():
>> if (force == TRUE&& (!token || token[0] == '\0'))
>>
>> libsvn_ra_serf\locks.c, line 329(1.6.x branch), function set_lock_headers():
>> if (lock_ctx->force == TRUE)
>>
>
> I would find it very counter-intuitive to change these to:
> if (lock_ctx->force != FALSE) ...
> ... and to ensure that people are going to remember a rule like this in
> the future.
>
... most of our TRUE checks are done as "if (lock_ctx->force)" though,
wouldn't have a problem with that.
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1753838
Re: boolean issue with serf
Posted by Lieven Govaerts <sv...@mobsol.be>.
On 04/16/2009 09:39 PM, Stefan Küng wrote:
> On Thu, Apr 16, 2009 at 21:30, Stefan Küng<to...@gmail.com> wrote:
..
>
> A quick search through the files revealed:
>
> libsvn_ra_serf\locks.c, line 720 (1.6.x branch), function svn_ra_serf__unlock():
> if (force == TRUE&& (!token || token[0] == '\0'))
>
> libsvn_ra_serf\locks.c, line 329(1.6.x branch), function set_lock_headers():
> if (lock_ctx->force == TRUE)
>
I would find it very counter-intuitive to change these to:
if (lock_ctx->force != FALSE) ...
... and to ensure that people are going to remember a rule like this in
the future.
On 04/16/2009 09:38 PM, C. Michael Pilato wrote:
>
> While I generally agree that for all intents and purposes, false == 0 and
> true is anything else, we do provide the values TRUE and FALSE for use with
> svn_boolean_t and it is not technically a contract violation to expect
> exactly one of those two values to be provided by the caller.
>
Agreed.
Lieven
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1753804
Re: boolean issue with serf
Posted by Stefan Küng <to...@gmail.com>.
On Thu, Apr 16, 2009 at 21:30, Stefan Küng <to...@gmail.com> wrote:
> Hi,
>
> A user reported a strange problem that happens only with TSVN and serf,
> but not with the CL client.
> http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=4061&dsMessageId=1752435
>
> After some testing I found that TSVN passes a value of 32 as 'true' to
> svn_client_lock for the --force flag. This works ok when using neon, but
> fails when using serf. If I pass 1 instead of 32 then serf works too.
>
> Now while I can fix this easily in TSVN, I'm wondering if it's really
> required to pass 1 as svn_boolean_t values? Shouldn't all comparisons be
> done as !=0 instead of == 1 in C since it doesn't have a bool type?
>
> I haven't stepped through the serf code to find out where the comparison
> is that fails if I pass 32 as 'TRUE' though...
A quick search through the files revealed:
libsvn_ra_serf\locks.c, line 720 (1.6.x branch), function svn_ra_serf__unlock():
if (force == TRUE && (!token || token[0] == '\0'))
libsvn_ra_serf\locks.c, line 329(1.6.x branch), function set_lock_headers():
if (lock_ctx->force == TRUE)
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1753705