[Spice-devel] [PATCH spice-server] style: Update style to include some C++ element

[Christophe de Dinechin]

Frediano Ziglio writes:

> These style are used by other SPICE projects like spice-streaming-agent.
> See discussion "Coding style and naming conventions for C++" at
> https://lists.freedesktop.org/archives/spice-devel/2018-January/041562.html.
>
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  docs/spice_style.txt | 38 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/docs/spice_style.txt b/docs/spice_style.txt
> index eb0e30ef..0e0028e9 100644
> --- a/docs/spice_style.txt
> +++ b/docs/spice_style.txt
> @@ -1,7 +1,7 @@
>  Spice project coding style and coding conventions
>  =================================================
>
> -Copyright (C) 2009-2016 Red Hat, Inc.
> +Copyright (C) 2009-2018 Red Hat, Inc.
>  Licensed under a Creative Commons Attribution-Share Alike 3.0
>  United States License (see http://creativecommons.org/licenses/by-sa/3.0/us/legalcode).
>
> @@ -379,3 +379,39 @@ Also in source (no header) files you must include `config.h` at the beginning so
>
>  #include "spice_server.h"

(Not in your patch) Re-reading the part about headers, it has everything
completely backwards! I suggest we re-discuss this.

>  ----
> +
> +C++
> +---
> +C++ follows C style if not specified otherwise.

I would add

The C++11 dialect is assumed by default. No attempts will be made to
restrict the code to older variants of C++. Using constructs allowed in
later variants of C++ will be allowed either:

- After the default C++ compiler for building the oldest supported RHEL
  version fully supports the corresponding variant of C++

- Under the appropriate preprocessor conditions, if the previous
  condition is not met and C++11 alternatives would have significant
  drawbacks


> +
> +Method names
> +~~~~~~~~~~~~
> +
> +Method names should use lower case and separate words with
> +underscores.

What about classes? I see below that you selected camelcase for classes,
but that is inconsistent with standard C++ classes such as 'string', or
with most types in C (e.g. ptrdiff_t, struct stat, etc)

Since it seems that consistency with C and with the standard C++ library
was the primary rationale for using snake-case, I would go for
snake-case for classes as well.

(Frankly, I really could not care less, I'm just trying to be consistent)

> +
> +
> +Namespaces
> +~~~~~~~~~~
> +
> +Namespaces should use lower case and separate words with underscores.
> +Namespace blocks should not increase indentation.
> +Namespaces can be nested and closure can be collapsed but for

I would not use the word "closure" here, which has a specific meaning in
computer science (as in lambda variable capture). What about:

Namespaces can be nested. Namespace closing brackets for nested
namespaces can be placed together on the same line, but for readability
reasons the closure should specify the namespace with a comment.

> +readability reasons the closure should specify the namespace with a
> +comment.
> +
> +[source,cpp]
> +----
> +namespace spice {
> +namespace streaming_agent {

There is a style inconsistency regarding the placement of the opening
brace relative to other first-column opening braces (function and
class). So I would suggest that all top-level opening braces be on a
line of their own, as for 'class' below and for functions.

> +
> +class ClassInsideNamespace
> +{
> +...
> +};
> +
> +}} // namespace spice::streaming_agent
> +----
> +
> +You should not import an entire namespace but use qualified names
> +instead.

The C++ terminology is "using namespace" and not "import". So I would
rephrase as:

The "using namespace" construct should never be used in headers. It should
be used sparingly in source files, and only within the body of short
functions.

Preferred alternatives to "using namespace" include:
- using declarations
        using spice::streaming_agent::some_class;
- namespace aliases
        namespace ssa = spice::streaming_agent;


--
Cheers,
Christophe de Dinechin (IRC c3d)