[Mesa-dev] [PATCH 3/3] docs: update the coding style information
Thomas Helland
thomashelland90 at gmail.com
Mon May 25 12:59:13 PDT 2015
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="http://www.stack.nl/~dimitri/doxygen/">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)
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?
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)
--Thomas
> -<p>
> -Local variable name example: localVarName (no underscores)
> -</p>
> -
> -<p>
> -Constants and macros are ALL_UPPERCASE, with _ between words
> -</p>
> -
> -<p>
> -Global variables are not allowed.
> -</p>
> -
> -<p>
> -Function name examples:
> -</p>
> +<li>Function names follow various conventions depending on the type of function:
> <pre>
> - glFooBar() - a public GL entry point (in glapi_dispatch.c)
> - _mesa_FooBar() - the internal immediate mode function
> - save_FooBar() - retained mode (display list) function in dlist.c
> - foo_bar() - a static (private) function
> - _mesa_foo_bar() - an internal non-static Mesa function
> + glFooBar() - a public GL entry point (in glapi_dispatch.c)
> + _mesa_FooBar() - the internal immediate mode function
> + save_FooBar() - retained mode (display list) function in dlist.c
> + foo_bar() - a static (private) function
> + _mesa_foo_bar() - an internal non-static Mesa function
> </pre>
>
> -<p>
> -Places that are not directly visible to the GL API should prefer the use
> -of <tt>bool</tt>, <tt>true</tt>, and
> +<li>Constants, macros and enumerant names are ALL_UPPERCASE, with _ between
> +words.
> +<li>Mesa usually uses camel case for local variables (Ex: "localVarname")
> +while gallium typically uses underscores (Ex: "local_var_name").
> +<li>Global variables are almost never used because Mesa should be thread-safe.
> +
> +<li>Booleans. Places that are not directly visible to the GL API
> +should prefer the use of <tt>bool</tt>, <tt>true</tt>, and
> <tt>false</tt> over <tt>GLboolean</tt>, <tt>GL_TRUE</tt>, and
> <tt>GL_FALSE</tt>. In C code, this may mean that
> <tt>#include <stdbool.h></tt> needs to be added. The
> <tt>try_emit_</tt>* methods in src/mesa/program/ir_to_mesa.cpp and
> src/mesa/state_tracker/st_glsl_to_tgsi.cpp can serve as examples.
> -</p>
> +
> +</ul>
>
>
> <h2 id="submitting">Submitting patches</h2>
> --
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list