[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1387696 - /subversion/trunk/subversion/libsvn_wc/conflicts.c

From: Hyrum K Wright <hyrum_at_hyrumwright.org>
Date: Thu, 11 Oct 2012 17:25:03 -0400

On Thu, Oct 11, 2012 at 3:01 PM, Stefan Sperling <stsp_at_elego.de> wrote:
> On Thu, Oct 11, 2012 at 12:20:32PM -0400, Hyrum K Wright wrote:
>> > + /* Compute the incoming side of the conflict ('action'). */
>> > + if (their_value == NULL)
>> > + desc->action = svn_wc_conflict_action_delete;
>> > + else if (my_value == NULL)
>> > + desc->action = svn_wc_conflict_action_add;
>>
>> Wrong enum type assignment here, the source is an
>> svn_wc_conflict_reason_t, but the destination is an
>> svn_wc_conflict_action_t.
>
> Really? I see neither svn_wc_conflict_action_delete nor
> svn_wc_conflict_action_add in the definition of svn_wc_conflict_reason_t.

Whoops, sorry, I should have referenced a different line below,
currently line 2003.

>
>> While this might work now, it feels like a
>> real opportunity for obscure bugs later. (And my compiler complains
>> about it. :) )
>>
>> > + else
>> > + desc->action = svn_wc_conflict_action_edit;
>>
>> Same.
>
> Seems fine to me. Both are saying 'action'... did you mean to
> quote a different part of the code?

Yes, I meant to quote a different part.

This is the compiler warning I see:
subversion/libsvn_wc/conflicts.c:2003:24: warning: implicit conversion from
      enumeration type 'enum svn_wc_conflict_reason_t' to different
enumeration type
      'svn_wc_conflict_action_t' (aka 'enum svn_wc_conflict_action_t')
      [-Wconversion]
        desc->action = svn_wc_conflict_reason_added;
                     ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
subversion/libsvn_wc/conflicts.c:2005:24: warning: implicit conversion from
      enumeration type 'enum svn_wc_conflict_reason_t' to different
enumeration type
      'svn_wc_conflict_action_t' (aka 'enum svn_wc_conflict_action_t')
      [-Wconversion]
        desc->action = svn_wc_conflict_reason_edited;
                     ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Does this (untested) patch make sense?

[[[
Index: subversion/libsvn_wc/conflicts.c
===================================================================
--- subversion/libsvn_wc/conflicts.c (revision 1397318)
+++ subversion/libsvn_wc/conflicts.c (working copy)
@@ -2000,9 +2000,9 @@
       if (my_value == NULL)
         desc->reason = svn_wc_conflict_reason_deleted;
       else if (their_value == NULL)
- desc->action = svn_wc_conflict_reason_added;
+ desc->reason = svn_wc_conflict_reason_added;
       else
- desc->action = svn_wc_conflict_reason_edited;
+ desc->reason = svn_wc_conflict_reason_edited;

       /* ### This should be changed. The prej file should be stored
        * ### separately from the other files. We need to rev the
]]]

-Hyrum
Received on 2012-10-11 23:25:36 CEST

This is an archived mail posted to the Subversion Dev mailing list.