[PATCH] lib: Fix whitespace in params.c

ronnie sahlberg ronniesahlberg at gmail.com
Mon Aug 11 10:55:28 MDT 2014


-Wall / -Wparentheses eliminates the need for that "trick" (which
reduces readability).

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