[Mesa-dev] [PATCH 3/3] docs: update the coding style information

[Brian Paul]

On 05/25/2015 01:59 PM, Thomas Helland wrote:
> 2015-05-25 18:20 GMT+02:00 Brian Paul <brianp at vmware.com>:
>> This hasn't been updated in a long time and from recent discussion on
>> the mailing list, it's not always clear what's expected.  Hopefully,
>> this will help a bit.
>> ---
>>   docs/devinfo.html | 155 ++++++++++++++++++++++++++++++------------------------
>>   1 file changed, 86 insertions(+), 69 deletions(-)
>>
>> diff --git a/docs/devinfo.html b/docs/devinfo.html
>> index a6fb76b..4ab8e4b 100644
>> --- a/docs/devinfo.html
>> +++ b/docs/devinfo.html
>> @@ -28,97 +28,114 @@
>>   <h2 id="style">Coding Style</h2>
>>
>>   <p>
>> -Mesa's code style has changed over the years.  Here's the latest.
>> +Mesa is over 20 years old and the coding style has evolved over time.
>> +Some old parts use a style that's a bit out of date.
>> +If the guidelines below don't cover something, try following the format of
>> +existing, neighboring code.
>>   </p>
>>
>>   <p>
>> -Comment your code!  It's extremely important that open-source code be
>> -well documented.  Also, strive to write clean, easily understandable code.
>> +Basic formatting guidelines
>>   </p>
>>
>> -<p>
>> -3-space indentation
>> -</p>
>> -
>> -<p>
>> -If you use tabs, set them to 8 columns
>> -</p>
>> +<ul>
>> +<li>3-space indentation, no tabs.
>> +<li>Limit lines to 78 or fewer characters.  The idea is to prevent line
>> +wrapping in 80-column editors and terminals.  There are exceptions, such
>> +as if you're defining a large, static table of information.
>> +<li>Opening braces go on the same line as the if/for/while statement.
>> +For example:
>> +<pre>
>> +   if (condition) {
>> +      foo;
>> +   } else {
>> +      bar;
>> +   }
>> +</pre>
>>
>> -<p>
>> -Line width: the preferred width to fill comments and code in Mesa is 78
>> -columns.  Exceptions are sometimes made for clarity (e.g. tabular data is
>> -sometimes filled to a much larger width so that extraneous carriage returns
>> -don't obscure the table).
>> -</p>
>> +<li>Put a space before/after operators.  For example, <tt>a = b + c;</tt>
>> +and not <tt>a=b+c;</tt>
>>
>> -<p>
>> -Brace example:
>> -</p>
>> +<li>This GNU indent command generally does the right thing for formatting:
>>   <pre>
>> -       if (condition) {
>> -          foo;
>> -       }
>> -       else {
>> -          bar;
>> -       }
>> -
>> -       switch (condition) {
>> -       case 0:
>> -          foo();
>> -          break;
>> -
>> -       case 1: {
>> -          ...
>> -          break;
>> -       }
>> -
>> -       default:
>> -          ...
>> -          break;
>> -       }
>> +   indent -br -i3 -npcs --no-tabs infile.c -o outfile.c
>>   </pre>
>>
>> -<p>
>> -Here's the GNU indent command which will best approximate my preferred style:
>> -(Note that it won't format switch statements in the preferred way)
>> -</p>
>> +<li>Use comments wherever you think it would be helpful for other developers.
>> +Several specific cases and style examples follow.  Note that we roughly
>> +follow <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__www.stack.nl_-7Edimitri_doxygen_&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8&m=PsoDvq4p3p1CKPfE1UV_uawQ3CZ0Z4qqIsPOp3mrKxo&s=ayuYzAhS4g_eIzw-agUVkTRzRxtLxNDBJPHZuoSTyr8&e= ">Doxygen</a> conventions.
>> +<br>
>> +<br>
>> +Single-line comments:
>>   <pre>
>> -       indent -br -i3 -npcs --no-tabs infile.c -o outfile.c
>> +   /* null-out pointer to prevent dangling reference below */
>> +   bufferObj = NULL;
>> +</pre>
>> +Or,
>> +<pre>
>> +   bufferObj = NULL;  /* prevent dangling reference below */
>> +</pre>
>> +Multi-line comment:
>> +<pre>
>> +   /* If this is a new buffer object id, or one which was generated but
>> +    * never used before, allocate a buffer object now.
>> +    */
>> +</pre>
>> +We try to quote the OpenGL specification where prudent:
>> +<pre>
>> +   /* Page 38 of the PDF of the OpenGL ES 3.0 spec says:
>> +    *
>> +    *     "An INVALID_OPERATION error is generated for any of the following
>> +    *     conditions:
>> +    *
>> +    *     * <length> is zero."
>> +    *
>> +    * Additionally, page 94 of the PDF of the OpenGL 4.5 core spec
>> +    * (30.10.2014) also says this, so it's no longer allowed for desktop GL,
>> +    * either.
>> +    */
>> +</pre>
>> +Function comment example:
>> +<pre>
>> +   /**
>> +    * Create and initialize a new buffer object.  Called via the
>> +    * ctx->Driver.CreateObject() driver callback function.
>> +    * \param  name  integer name of the object
>> +    * \param  type  one of GL_FOO, GL_BAR, etc.
>> +    * \return  pointer to new object or NULL if error
>> +    */
>> +   struct gl_object *
>> +   _mesa_create_object(GLuint name, GLenum type)
>>   </pre>
>>
>> +<li>Put the function return type and qualifiers on one line and the function
>> +name and parameters on the next, as seen above.  This makes it easy to use
>> +<code>grep ^function_name dir/*</code> to find function definitions.
>>
>
> Maybe add that we put the opening brace on a new line for functions?
> (and include that in the function definition example above)

Done.


> This kinda confused me at the beginning;
> why put the brace on the same line as if's/loops,
> but on its own line for functions?

It's a convention used by many other projects, like Linux.  I think it 
goes back to the K&R book.  I think of it as top-level constructs 
(functions, structs, enums) having the opening brace on its own line.


> I got some nits due to that when I first started mesa-hacking.
> Having it stated here may just help others avoid that.
> It's your call, I don't have a strong opinion.
>
> I'm no expert on how to add extensions, but patch 2 and 3,
> and the html of patch one LGTM. (Apart from what Ilia pointed out)

Is that a R-b?

-Brian