On 03/06/2017 05:04 AM, Daniel P. Berrange wrote:
On Mon, Mar 06, 2017 at 10:21:28AM +0100, Bjoern Walk wrote:
> Daniel P. Berrange <berrange(a)redhat.com> [2017-03-03, 10:50AM +0100]:
>> This documents the preferred conventions for naming files,
>> structs, enums, typedefs and functions.
>>
>> Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
>> ---
>> HACKING | 71 ++++++++++++++++++++++++++++++++++++++++++++
>> docs/hacking.html.in | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> docs/hacking2.xsl | 4 +++
>> 3 files changed, 158 insertions(+)
>>
>> diff --git a/HACKING b/HACKING
>> index fff003b..16be5cf 100644
>> --- a/HACKING
>> +++ b/HACKING
>> @@ -239,6 +239,77 @@ on the subject, on Richard Jones' guide to working with
open source projects
>>
<
http://people.redhat.com/rjones/how-to-supply-code-to-open-source-project...;.
>>
>>
>> +Naming conventions
>> +==================
>> +When reading libvirt code, a number of different naming conventions will be
>> +evident due to various changes in thinking over the course of the project's
>> +lifetime. The conventions documented below should be followed when creating
>> +any entirely new files in libvirt. When working on existing files, while it is
>> +desirable to apply these conventions, keeping a consistent style with existing
>> +code in that particular file is generally more important. The overall guiding
>> +rule is that every file, enum, struct, function, and typedef name must have a
>> +'vir' or 'VIR' prefix. All local scope variable names are
exempt, and global
>> +variables are exempt, unless exported in a header file.
>> +
>> +*File names*
>> +
>> +File naming varies depending on the subdirectory. The preferred style is to
>> +have a 'vir' prefix, followed by a name which matches the name of the
>> +functions / objects inside the file. For example, a file containing an object
>> +'virHashtable' is stored in files 'virhashtable.c' and
'virhashtable.h'.
>> +Sometimes, methods which would otherwise be declared 'static' need to
be
>> +exported for use by a test suite. For this purpose a second header file should
>> +be added with a suffix of 'priv'. e.g. 'virhashtablepriv.h'. USe
of
>> +underscores in file names is discouraged when using the 'vir' prefix
style.
>> +The 'vir' prefix naming applies to src/util, src/rpc and tests/
directories.
>> +Most other directories do not follow this convention.
>> +
>> +
>> +
>> +*Enum type & field names*
>> +
>> +All enums should have a 'vir' prefix in their typedef name, and each
following
>> +word should have its first letter in uppercase. The enum name should match the
>> +typedef name with a leading underscore. The enum member names should be in all
>> +uppercase, and use an underscore to separate each word. The enum member name
>> +prefix should match the enum typedef name.
>> +
>> + typedef enum _virSocketType virSocketType;
>> + enum _virSocketType {
>> + VIR_SOCKET_TYPE_IPV4,
>> + VIR_SOCKET_TYPE_IPV6,
>> + };
>> +
>> +
>> +*Struct type names*
>> +
>> +All structs should have a 'vir' prefix in their typedef name, and each
>> +following word should have its first letter in uppercase. The struct name
>> +should be the same as the typedef name with a leading underscore. A second
>> +typedef should be given for a pointer to the struct with a 'Ptr'
suffix.
>> +
>> + typedef struct _virHashTable virHashTable;
>> + typedef virHashTable *virHashTablePtr;
>> + struct _virHashTable {
>> + ...
>> + };
>> +
> I personally would prefer this style:
>
> typedef struct _virHashTable {
> ...
> } virHashTable, *virHashTablePtr;
> This is done for example in src/conf/device_conf.h. Subjectively, it is
> much easier to read, but objectively, it is more concise and enhances
> discoverability. For example, in src/conf/domain_conf.h the typedef are
> at the beginning of the file separated from the definition of the
> struct. If I want to look up a virDomainDiskDefPtr it requires two
> jumps.
We should change device_conf.h really - it is different from pretty much
everywhere else in the libvirt codebase.
There are others too. I've always disliked the separate typedefs and
extra _virBlah struct name, so quite awhile back I posted some patches
that used the more compact style in new struct definitions as a way of
suggesting that we use that instead, and they passed review. So I did
several more that way, and those passed as well (although I do remember
one dissenting opinion for one patch).
But if we're going to formalize struct definitions in a coding standards
document, then I'm willing to throw in the towel - to avoid leading
unsuspecting copy-pasters in the wrong direction, I just sent a patch
that changes all "compact format" struct definitions to the more verbose
format in the document.