[Mesa-dev] [PATCH] gallium: Formatted code in pb_buffer_fenced.c according to style guide.

[Brian Paul]

On 03/24/2016 07:03 PM, Thomas Helland wrote:
> Welcome to the community!
>
> Some small comments.
> We usually write patch messages in present tense.
> So, "Format code in .... ".
> Some more comments below.
>
> On Mar 25, 2016 01:24, "Rovanion Luckey" <rovanion.luckey at gmail.com
> <mailto:rovanion.luckey at gmail.com>> wrote:
>  >
>  > This is a tiny housekeeping patch which does the following:
>  >
>  >         * Replaced tabs with three spaces.
>  >         * Formatted oneline and multiline code comments. Some doxygen
>  >                 comments weren't marked as such and some code
> comments were marked
>  >                 as doxygen comments.
>  >         * Spaces between if- and while-statements and their parenthesis.
>  >
>  > As specified on: http://mesa3d.org/devinfo.html#style
>  >
>
> Nice detailed commit message!
> I'm not sure, but I think you should just write:
> "According to coding style standards" or something.
> Links may die, and so having them in the commit message
> may not give us much in the future.
>
>  > The only interesting point would be @@ -364,14 +363,9 @@ where the
>  > following seemingly trivial change is applied.
>  >
>  > -         boolean destroyed;
>  > -
>  > -         destroyed = fenced_buffer_remove_locked(fenced_mgr,
> fenced_buf);
>  > +         boolean destroyed = fenced_buffer_remove_locked(fenced_mgr,
> fenced_buf);
>  >
>  > It may be that I'm missing some of the finer points of C making this
>  > into a semantic change instead of the only syntactic change I was
>  > after, in which case the change should be removed. It might also be
>  > that it should be removed from this change set either way since it
>  > could be considered non-trivial.
>  >
>
> I'm not sure how this works now, but I believe there was
> something with older versions of MSVC that didn't allow
> initializing variables like this, and that's why it is separated
> in declaration and initialization.
> I believe C89 was the thing here? The VMware guys will know.

IIRC, the problem was with named initializers for structures.  I don't 
think it's an issue now that we require Visual Studio 2013.

Ordinary, simple initializers like above are always fine.


> I believe the requirement was lifted to a newer C standard(C99?).
> Therefore this is now OK I believe.
>
> I haven't looked to closely at the other changes.
> I can do that tomorrow, if no one gets to it before me.
>
> Don't let the minor nitpicking get you down ;-)
> There's always the occasional nitpick when you first adapt
> to how things are done in the project.
>
> Thanks for your first patch!

Yes.

-Brian