[PATCH] svn/conflict-callbacks.c indentation fix

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] svn/conflict-callbacks.c indentation fix

Joe Orton
There are two gcc 8.x (I'm using 8.2.1) warnings from which catch those:

  if (foo)
    bar;
    baz;

type of errors.  I fixed one case which looks obviously like a false
positive in r1845556, but I'm not sure about the other one, in this
code:

http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/conflict-callbacks.c?view=markup#l1528

The warning is:

subversion/svn/conflict-callbacks.c: In function ‘build_tree_conflict_options’:
subversion/svn/conflict-callbacks.c:1531:7: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
       if (all_options_are_dumb != NULL &&
       ^~
subversion/svn/conflict-callbacks.c:1537:9: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘if’
         if (*possible_moved_to_repos_relpaths == NULL)

Can someone familiar with the code check whether it's OK?

[[[
* subversion/svn/conflict-callbacks.c
  (build_tree_conflict_options): No functional change; fix
  indentation to avoid gcc -Wmisleading-indentation warning.
]]]


svn_cc_reindent.diff (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] svn/conflict-callbacks.c indentation fix

Stefan Sperling
On Fri, Nov 02, 2018 at 09:39:47AM +0000, Joe Orton wrote:

> There are two gcc 8.x (I'm using 8.2.1) warnings from which catch those:
>
>   if (foo)
>     bar;
>     baz;
>
> type of errors.  I fixed one case which looks obviously like a false
> positive in r1845556, but I'm not sure about the other one, in this
> code:
>
> http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/conflict-callbacks.c?view=markup#l1528
>
> The warning is:
>
> subversion/svn/conflict-callbacks.c: In function ‘build_tree_conflict_options’:
> subversion/svn/conflict-callbacks.c:1531:7: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
>        if (all_options_are_dumb != NULL &&
>        ^~
> subversion/svn/conflict-callbacks.c:1537:9: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘if’
>          if (*possible_moved_to_repos_relpaths == NULL)
>
> Can someone familiar with the code check whether it's OK?

Hi Joe,

I confirm that your patch is correct. I mis-indented these blocks.

> [[[
> * subversion/svn/conflict-callbacks.c
>   (build_tree_conflict_options): No functional change; fix
>   indentation to avoid gcc -Wmisleading-indentation warning.
> ]]]
>

> Index: subversion/svn/conflict-callbacks.c
> ===================================================================
> --- subversion/svn/conflict-callbacks.c (revision 1845554)
> +++ subversion/svn/conflict-callbacks.c (working copy)
> @@ -1534,16 +1534,16 @@
>            id != svn_client_conflict_option_accept_current_wc_state)
>          *all_options_are_dumb = FALSE;
>  
> -        if (*possible_moved_to_repos_relpaths == NULL)
> -          SVN_ERR(
> -            svn_client_conflict_option_get_moved_to_repos_relpath_candidates2(
> -              possible_moved_to_repos_relpaths, builtin_option,
> -              result_pool, iterpool));
> +      if (*possible_moved_to_repos_relpaths == NULL)
> +        SVN_ERR(
> +          svn_client_conflict_option_get_moved_to_repos_relpath_candidates2(
> +            possible_moved_to_repos_relpaths, builtin_option,
> +            result_pool, iterpool));
>  
> -        if (*possible_moved_to_abspaths == NULL)
> -          SVN_ERR(svn_client_conflict_option_get_moved_to_abspath_candidates2(
> -                    possible_moved_to_abspaths, builtin_option,
> -                    result_pool, iterpool));
> +      if (*possible_moved_to_abspaths == NULL)
> +        SVN_ERR(svn_client_conflict_option_get_moved_to_abspath_candidates2(
> +                  possible_moved_to_abspaths, builtin_option,
> +                  result_pool, iterpool));
>      }
>  
>    svn_pool_destroy(iterpool);

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] svn/conflict-callbacks.c indentation fix

Branko Čibej
On 02.11.2018 10:44, Stefan Sperling wrote:

> On Fri, Nov 02, 2018 at 09:39:47AM +0000, Joe Orton wrote:
>> There are two gcc 8.x (I'm using 8.2.1) warnings from which catch those:
>>
>>   if (foo)
>>     bar;
>>     baz;
>>
>> type of errors.  I fixed one case which looks obviously like a false
>> positive in r1845556, but I'm not sure about the other one, in this
>> code:
>>
>> http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/conflict-callbacks.c?view=markup#l1528
>>
>> The warning is:
>>
>> subversion/svn/conflict-callbacks.c: In function ‘build_tree_conflict_options’:
>> subversion/svn/conflict-callbacks.c:1531:7: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
>>        if (all_options_are_dumb != NULL &&
>>        ^~
>> subversion/svn/conflict-callbacks.c:1537:9: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘if’
>>          if (*possible_moved_to_repos_relpaths == NULL)
>>
>> Can someone familiar with the code check whether it's OK?
> Hi Joe,
>
> I confirm that your patch is correct. I mis-indented these blocks.

Time to start using a real editor, I guess? :)

-- Brane

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] svn/conflict-callbacks.c indentation fix

Joe Orton
In reply to this post by Stefan Sperling
On Fri, Nov 02, 2018 at 10:44:33AM +0100, Stefan Sperling wrote:
> I confirm that your patch is correct. I mis-indented these blocks.

Great, thank you for checking, Stefan.  Committed in r1845559.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] svn/conflict-callbacks.c indentation fix

Stefan Sperling
In reply to this post by Branko Čibej
On Fri, Nov 02, 2018 at 10:49:23AM +0100, Branko Čibej wrote:
> On 02.11.2018 10:44, Stefan Sperling wrote:
> > I confirm that your patch is correct. I mis-indented these blocks.
>
> Time to start using a real editor, I guess? :)

The further I progress into adulthood, the more our tiny indentation
by two spaces seems to be causing me trouble...

Maybe(we) could have another (historical) vote, on tabs vs spaces this time?
Disclaimer: I'd vote for tabs :)
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] svn/conflict-callbacks.c indentation fix

Greg Stein-4
On Fri, Nov 2, 2018 at 5:08 AM Stefan Sperling <[hidden email]> wrote:
On Fri, Nov 02, 2018 at 10:49:23AM +0100, Branko Čibej wrote:
> On 02.11.2018 10:44, Stefan Sperling wrote:
> > I confirm that your patch is correct. I mis-indented these blocks.
>
> Time to start using a real editor, I guess? :)

The further I progress into adulthood, the more our tiny indentation
by two spaces seems to be causing me trouble...

Maybe(we) could have another (historical) vote, on tabs vs spaces this time?
Disclaimer: I'd vote for tabs :)

Heresy!!!
 
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] svn/conflict-callbacks.c indentation fix

James McCoy-3
In reply to this post by Stefan Sperling
On Fri, Nov 02, 2018 at 11:08:10AM +0100, Stefan Sperling wrote:

> On Fri, Nov 02, 2018 at 10:49:23AM +0100, Branko Čibej wrote:
> > On 02.11.2018 10:44, Stefan Sperling wrote:
> > > I confirm that your patch is correct. I mis-indented these blocks.
> >
> > Time to start using a real editor, I guess? :)
>
> The further I progress into adulthood, the more our tiny indentation
> by two spaces seems to be causing me trouble...
>
> Maybe(we) could have another (historical) vote, on tabs vs spaces this time?
> Disclaimer: I'd vote for tabs :)

Or always use {}, even for 1 line blocks.

Cheers,
--
James
GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7  2D23 DFE6 91AE 331B A3DB
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] svn/conflict-callbacks.c indentation fix

Branko Čibej
On 02.11.2018 11:33, James McCoy wrote:

> On Fri, Nov 02, 2018 at 11:08:10AM +0100, Stefan Sperling wrote:
>> On Fri, Nov 02, 2018 at 10:49:23AM +0100, Branko Čibej wrote:
>>> On 02.11.2018 10:44, Stefan Sperling wrote:
>>>> I confirm that your patch is correct. I mis-indented these blocks.
>>> Time to start using a real editor, I guess? :)
>> The further I progress into adulthood, the more our tiny indentation
>> by two spaces seems to be causing me trouble...
>>
>> Maybe(we) could have another (historical) vote, on tabs vs spaces this time?
>> Disclaimer: I'd vote for tabs :)
> Or always use {}, even for 1 line blocks.

No space after bang! No space after bang! Ye who write 'if (! foo) ...'
with a space after the bang shall be forever cast into the void! Yaargh!

-- Brane

P.S.: This is shaping up to become a fun thread, heh heh.