Fixing up the code style

Erich E. Hoover erich.e.hoover at gmail.com
Sat Feb 15 00:32:42 CST 2014


On Thu, Feb 13, 2014 at 7:24 AM, Stefan Dösinger
<stefandoesinger at gmail.com> wrote:
> ...
> Maybe, or maybe not. The basic issue still stands. New contributors
> will look at the existing code, and if its inconsistent how are they
> supposed to write code that's consistent in itself. That's not a
> question of pickiness but approach to the problem.
> ...

I'm not sure it's always clear what to do even the case of more
experienced contributors, I've been taking a look at fixing a double
unlock bug in RtlAcquireResourceExclusive (
http://source.winehq.org/source/dlls/ntdll/rtl.c#L157 ).  And the
style there indent-wise is so screwy that it's not clear what I should
do in my patch.  This one function currently has (level wise):
1) Four space indents
2) A tab indent + a space indent (in some cases just the tab)
3) A tab indent + a five space indent (alternatively, 13 spaces)
4) Two tabs + a space indent

So, what do I do?  Do I just change line 190 to fix the bug and keep
the two tab + a space indent?  Do I fix just this line to conform to
the standard since it's the only one I need to touch?  Do I fix this
one function because a lot of it doesn't conform to the standard?
Personally, I would like to have a separate formatting patch to fix
the function so that it's clear that I'm just fixing the one thing -
but that's expressly prohibited under the current guidelines.

> ...
> My impression was that the answers to that were no and no, with the
> idea that the problem will be fixed by unifying the style together
> with other, functional changes. That way we end up with picky reviews
> and IMO that approach is bad.
> ...

I agree, Sebastian and I have been debating what to do with this
function (and RtlAcquireResourceShared) for some time and style is the
only thing keeping us from submitting the one-line fix for these.
Honestly, it would be really great if we could run something like
astyle on the whole codebase so that we were always working off of a
"known good" style.  If we could get that implemented then we could
potentially think about verifying the patch style on submission and
auto-rejecting patches that have style problems.  Personally, I think
that would be an amazing improvement to our submission process, but I
have to use K&R style for work and I hate feeling like an idiot when I
slip into the wrong style for a project.

Best,
Erich



More information about the wine-devel mailing list