Re: [Spice-devel] [PATCH v3 09/11] Add mention of header guards

[Christophe de Dinechin]

> On 14 Feb 2018, at 16:43, Christophe Fergeau  wrote:
> 
> On Thu, Feb 08, 2018 at 12:25:29PM +0100, Christophe de Dinechin wrote:
>> From: Christophe de Dinechin 
>> 
>> Signed-off-by: Christophe de Dinechin 
>> ---
>> docs/spice_style.txt | 19 +++
>> 1 file changed, 19 insertions(+)
>> 
>> diff --git a/docs/spice_style.txt b/docs/spice_style.txt
>> index eb2ee252..ae91f987 100644
>> --- a/docs/spice_style.txt
>> +++ b/docs/spice_style.txt
>> @@ -385,6 +385,25 @@ char *array[] = {
>> "item_3",
>> };
>> 
>> +Headers
>> +---
>> +
>> +Headers should be protected against multiple inclusion using a macro that 
>> contains the header file name in uppercase, with all characters that are 
>> invalid in C replaced with an underscore '_':
>> +
>> +[source,c]
>> +---
>> +#ifndef MY_MODULE_H
>> +#define MY_MODULE_H
>> +
>> +...
>> +
>> +#endif // MY_MODULE_H
> 
> I see this has been pushed, but this does not match what server/ is
> doing, so nack from me.

There was an agreement ahead of time that we we updating the guidelines, not 
looking at how it was done.

> 
>> +---
>> +
>> +The macro may include additional information, e.g. a component. For example 
>> a file generally referenced as foo/bar.h could use a FOO_BAR_H macro.
>> +
>> +Historically, some headers added underscores liberally, e.g. MY_MODULE_H_. 
>> This is neither necessary nor discouraged, although as a reminder, a leading 
>> underscore followed by a capital letter is reserved for the implementation 
>> and should not be used, so _MY_MODULE_H is, technically speaking, invalid C.
> 
> And really, no to these "oh, we have a standard, but you can make up
> your own variations around it". I would not even mention the historical
> stuff, or just say that it's obsolete, and ideally should be fixed.

We can fix it, or just acknowledge harmless past practices. Acknowledging is 
more economical, and in that case, nobody really cares.


Christophe
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH v3 09/11] Add mention of header guards

[Christophe Fergeau]

On Thu, Feb 08, 2018 at 12:25:29PM +0100, Christophe de Dinechin wrote:
> From: Christophe de Dinechin 
> 
> Signed-off-by: Christophe de Dinechin 
> ---
>  docs/spice_style.txt | 19 +++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/docs/spice_style.txt b/docs/spice_style.txt
> index eb2ee252..ae91f987 100644
> --- a/docs/spice_style.txt
> +++ b/docs/spice_style.txt
> @@ -385,6 +385,25 @@ char *array[] = {
>  "item_3",
>  };
>  
> +Headers
> +---
> +
> +Headers should be protected against multiple inclusion using a macro that 
> contains the header file name in uppercase, with all characters that are 
> invalid in C replaced with an underscore '_':
> +
> +[source,c]
> +---
> +#ifndef MY_MODULE_H
> +#define MY_MODULE_H
> +
> +...
> +
> +#endif // MY_MODULE_H

I see this has been pushed, but this does not match what server/ is
doing, so nack from me.

> +---
> +
> +The macro may include additional information, e.g. a component. For example 
> a file generally referenced as foo/bar.h could use a FOO_BAR_H macro.
> +
> +Historically, some headers added underscores liberally, e.g. MY_MODULE_H_. 
> This is neither necessary nor discouraged, although as a reminder, a leading 
> underscore followed by a capital letter is reserved for the implementation 
> and should not be used, so _MY_MODULE_H is, technically speaking, invalid C.

And really, no to these "oh, we have a standard, but you can make up
your own variations around it". I would not even mention the historical
stuff, or just say that it's obsolete, and ideally should be fixed.

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH v3 09/11] Add mention of header guards

[Frediano Ziglio]

> 
> From: Christophe de Dinechin 
> 
> Signed-off-by: Christophe de Dinechin 
> ---
>  docs/spice_style.txt | 19 +++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/docs/spice_style.txt b/docs/spice_style.txt
> index eb2ee252..ae91f987 100644
> --- a/docs/spice_style.txt
> +++ b/docs/spice_style.txt
> @@ -385,6 +385,25 @@ char *array[] = {
>  "item_3",
>  };
>  
> +Headers
> +---
> +
> +Headers should be protected against multiple inclusion using a macro that
> contains the header file name in uppercase, with all characters that are
> invalid in C replaced with an underscore '_':
> +
> +[source,c]
> +---
> +#ifndef MY_MODULE_H
> +#define MY_MODULE_H
> +
> +...
> +
> +#endif // MY_MODULE_H
> +---
> +
> +The macro may include additional information, e.g. a component. For example
> a file generally referenced as foo/bar.h could use a FOO_BAR_H macro.
> +
> +Historically, some headers added underscores liberally, e.g. MY_MODULE_H_.
> This is neither necessary nor discouraged, although as a reminder, a leading
> underscore followed by a capital letter is reserved for the implementation
> and should not be used, so _MY_MODULE_H is, technically speaking, invalid C.
> +
>  Header inclusion
>  
>  

Acked-by: Frediano Ziglio 

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH v3 09/11] Add mention of header guards

[Christophe de Dinechin]

From: Christophe de Dinechin 

Signed-off-by: Christophe de Dinechin 
---
 docs/spice_style.txt | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/docs/spice_style.txt b/docs/spice_style.txt
index eb2ee252..ae91f987 100644
--- a/docs/spice_style.txt
+++ b/docs/spice_style.txt
@@ -385,6 +385,25 @@ char *array[] = {
 "item_3",
 };
 
+Headers
+---
+
+Headers should be protected against multiple inclusion using a macro that 
contains the header file name in uppercase, with all characters that are 
invalid in C replaced with an underscore '_':
+
+[source,c]
+---
+#ifndef MY_MODULE_H
+#define MY_MODULE_H
+
+...
+
+#endif // MY_MODULE_H
+---
+
+The macro may include additional information, e.g. a component. For example a 
file generally referenced as foo/bar.h could use a FOO_BAR_H macro.
+
+Historically, some headers added underscores liberally, e.g. MY_MODULE_H_. 
This is neither necessary nor discouraged, although as a reminder, a leading 
underscore followed by a capital letter is reserved for the implementation and 
should not be used, so _MY_MODULE_H is, technically speaking, invalid C.
+
 Header inclusion
 
 
-- 
2.13.5 (Apple Git-94)

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel