[PATCH] lib: Fix whitespace in params.c

Christopher R. Hertel crh at ubiqx.mn.org
Mon Aug 11 10:58:31 MDT 2014


On 08/11/2014 11:55 AM, ronnie sahlberg wrote:
> -Wall / -Wparentheses eliminates the need for that "trick"

Agreed.

> (which reduces readability).

Matter of opinion.

Chris -)-----
"Coding style is very personal" -- Linus Torvalds  :)

>
> On Mon, Aug 11, 2014 at 9:37 AM, Christopher R. Hertel <crh at ubiqx.mn.org> wrote:
>> That particular file hasn't been modified in more than two years.
>> Modifying the formating at this point would not (IMHO) lose significant
>> history, and would bring that file into compliance with current project
>> standards.
>>
>> I do find it concerning that our current formatting style isn't supported by
>> indent.  How about astyle?  Shouldn't we be using a style that is standard
>> enough that there is support for it?  (Whitesmith, by the way, is supported
>> by astyle.)
>>
>> Volker:  The use of ( NULL==ptr ) vs. ( ptr==NULL ) is a common trick
>>           used to help catch forgotten equal signs.  ( NULL=ptr ) will
>>           cause a compiler error while ( ptr=NULL ) is a valid assignment.
>>           ...but that's just an explanation.  I'm not advocating for it.
>>
>> :)
>>
>> Chris -)-----
>>
>>
>> On 08/11/2014 09:02 AM, Michael Adam wrote:
>>>
>>> Chris,
>>>
>>> On 2014-08-11 at 07:31 -0500, Christopher R. Hertel wrote:
>>>>
>>>> On 08/11/2014 05:47 AM, Volker Lendecke wrote:
>>>>>
>>>>>
>>>>> Review&push would be appreciated.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Volker
>>>>>
>>>>
>>>> -1
>>>>
>>>> If you are going to change the formatting style of a file, run it through
>>>> indent and change the whole thing to match the project "standard".
>>>
>>>
>>> Absolutely no:
>>>
>>> A mere white-space change is by far not as intrusive as a
>>> general reformatting. As Volker's commit message indicates,
>>> you can verify with "git show -w" that this commit does not
>>> change anything (except for white spaces).
>>>
>>> Also, I don't believe that "indent" has a mode to fully match our
>>> README.coding guidelines.
>>>
>>>> Also, this isn't a "fix", it's just a change.  (This isn't a bug, it's a
>>>> preference.)
>>>
>>>
>>> Well it is not a fix of a code bug, but of a deficiency of that code
>>> file, namely the deficiency of this code file to use the desired
>>> formatting with respect to white spaces.
>>>
>>>> The formatting style in this particular file is known as
>>>> "whitesmith", and it was once quite popular.  I know, because it's my
>>>> coding
>>>> style.  I also know that I haven't touched that file since I rewrote it
>>>> for
>>>> Samba 2 more than 10 years ago.
>>>
>>>
>>> Note that the pure proposal of the patch does not imply any
>>> criticism of the author of the orignal file or the code itself.
>>> It just tries to adapt the file more to today's standards in
>>> Samba in the least obtrusive way.
>>>
>>>> So, my vote is to either leave it as is or to completely convert it to
>>>> the
>>>> current preferred formatting style.
>>>
>>>
>>> The disadvantage of that would be that it really disturbs the
>>> history. Git can detect (and ignore) whitespace changes, but
>>> it can't detect mere reformattings that go beyond that.
>>> (Git blame works across whitespace changes, not more intrusive
>>> reformattings.)
>>>
>>> It has hence been a guideline we followed for quite a while
>>> now that mere whitespace changes are OK, but reformatting
>>> patches beyond that are not.
>>>
>>> Hence I'd -1 committing an "indent" run on that file.
>>> So where are we? Leaving that file as it is? ... :-)
>>>
>>> Cheers - Michael
>>>
>>
>



More information about the samba-technical mailing list