qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH v2] CODING_STYLE.rst: flesh out our naming conventions.


From: Cornelia Huck
Subject: Re: [PATCH v2] CODING_STYLE.rst: flesh out our naming conventions.
Date: Tue, 11 Aug 2020 18:06:38 +0200

On Tue, 11 Aug 2020 17:55:08 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> Hi Alex,
> 
> On 8/10/20 12:51 PM, Alex Bennée wrote:
> > Mention a few of the more common naming conventions we follow in the
> > code base including common variable names and function prefix and
> > suffix examples.
> > 
> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> > 
> > ---  
> ...
> > +Function Naming Conventions
> > +---------------------------
> > +
> > +The ``qemu_`` prefix is used for utility functions that are widely
> > +called from across the code-base. This includes wrapped versions of
> > +standard library functions (e.g. qemu_strtol) where the prefix is
> > +added to the function name to alert readers that they are seeing a
> > +wrapped version; otherwise avoid this prefix.
> > +
> > +If there are two versions of a function to be called with or without a
> > +lock held, the function that expects the lock to be already held
> > +usually uses the suffix ``_locked``.  
> 
> And if there is only one version? I'm looking at:
> 
>   /* With q->lock */
>   static void nvme_kick(NVMeQueuePair *q)
>   {
>   ...
>   }
> 
> Should the style be enforced here and this function renamed
> nvme_kick_locked()?
> 
> In this particular case, I think so, because we also have:
> 
>   /* With q->lock */
>   static void nvme_put_free_req_locked(...)
>   {
>   ...
>   }
> 
>   /* With q->lock */
>   static void nvme_wake_free_req_locked(NVMeQueuePair *q)
>   {
>   ...
>   }
> 
> For more cases:
> 
> $ git grep -A1 -i '\/\*.*with.*lock'
> 
> 

I'm not sure we really want to encode calling conventions into function
names, beyond being able to distinguish between lock/no-lock versions.
Just appending _locked does not really tell us *which* lock is supposed
to be held, that needs to be documented in a comment anyway.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]