Re: [libvirt PATCH v2 1/2] docs: coding-style: Clarify on virXXXPtr types

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 2022-01-17 at 11:40 +0100, Peter Krempa wrote:
> On Mon, Jan 17, 2022 at 11:19:02 +0100, Tim Wiederhake wrote:
> > This partially reverts commit 9ccbed6afb.
> > 
> > Signed-off-by: Tim Wiederhake <twiederh@xxxxxxxxxx>
> > ---
> >  docs/coding-style.rst | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/docs/coding-style.rst b/docs/coding-style.rst
> > index 37e6009db4..ee4d551805 100644
> > --- a/docs/coding-style.rst
> > +++ b/docs/coding-style.rst
> > @@ -53,10 +53,15 @@ 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.
> > +   name with a leading underscore. For types that are part of the
> > +   public API, a second typedef should be given for a pointer to
> > +   the struct with a 'Ptr' suffix. Do not introduce new such
> > +   typedefs for internal types.
> > +
> >     ::
> >  
> >       typedef struct _virHashTable virHashTable;
> > +     typedef virHashTable *virHashTablePtr;
> 
> IMO this is wrong. 'virHashTable' is (or rather was) an internal type
> so
> the 'virHashTablePtr' version must not be defined for it.
> 
> The example should make a clear distinction if you want to add the
> caveat about public types.
> 
> Additionally virHashTable doesn't exist any more. I can fix that bit
> if
> you think it's out of scope of your patches since I'm the one who
> removed that.
> 
> Ideally we use some fake types which won't run the risk of being
> refactored later.
> 

Good point, virHashTablePtr is a poor example. Will replace with a fake
type as suggested.

> >       struct _virHashTable {
> >           ...
> >       };
> > -- 
> > 2.31.1
> > 
> 





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux