qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Cornelia Huck <cohuck@redhat.com>
To: "Philippe Mathieu-Daudé" <philmd@redhat.com>
Cc: "Alex Bennée" <alex.bennee@linaro.org>, qemu-devel@nongnu.org
Subject: Re: [PATCH v2] CODING_STYLE.rst: flesh out our naming conventions.
Date: Tue, 11 Aug 2020 18:06:38 +0200	[thread overview]
Message-ID: <20200811180638.7323b2bd.cohuck@redhat.com> (raw)
In-Reply-To: <3dfae8bc-7f8d-1b1c-c805-5eab4e8fbb19@redhat.com>

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	other threads:[~2020-08-11 16:08 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-10 10:51 [PATCH v2] CODING_STYLE.rst: flesh out our naming conventions Alex Bennée
2020-08-11  7:08 ` Cornelia Huck
2020-08-11 11:48   ` Alex Bennée
2020-08-11 11:56     ` Cornelia Huck
2020-08-11 15:55 ` Philippe Mathieu-Daudé
2020-08-11 16:06   ` Cornelia Huck [this message]
2020-08-23  8:20 ` Thomas Huth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200811180638.7323b2bd.cohuck@redhat.com \
    --to=cohuck@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=philmd@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).