From: Zhao Liu <zhao1.liu@intel.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org, qemu-devel@nongnu.org, qemu-rust@nongnu.org,
Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH 3/3] rust: pl011: Check size of state struct at compile time
Date: Fri, 21 Mar 2025 12:31:56 +0800 [thread overview]
Message-ID: <Z9zrvF7qRoykMupa@intel.com> (raw)
In-Reply-To: <CAFEAcA_BQOtzugW31ke=sit1mKnvxieGC_GXLOG4=MK_O2mKqw@mail.gmail.com>
> > > +// Some C users of this device embed its state struct into their own
> > > +// structs, so the size of the Rust version must not be any larger
> > > +// than the size of the C one. If this assert triggers you need to
> > > +// expand the padding_for_rust[] array in the C PL011State struct.
> > > +static_assert!(mem::size_of::<PL011State>() <= mem::size_of::<bindings::PL011State>());
> > > +
> >
> > maybe use qemu_api::bindings::PL011State directly? Because bindings
> > contains native C structures/functions and their use should not be
> > encouraged, I think it's better to 'hide' bindings (not list it at the
> > beginning of the file).
>
> Yeah, I wasn't sure what our preferred style approach is here
> regarding what we "use" and what we just directly reference
> (and the same in the other direction for mem::size_of vs
> size_of). Is there a "normal" pattern to follow here ?
There seems no clear doc on when to list use statements, but it's common
to list as clearly as possible to make it easier to sort out dependencies.
About bindings, I think it's better to clearly point out the specific
members in bindings, so ‘use qemu_api::bindings’ looks vague. Alternatively,
the qemu_api::bindings::PL011State could also be listed at the beginning of
the file, similar to a previous clean up: 06a1cfb5550a ("rust/pl011: Avoid
bindings::*") and another patch [1].
[1]: https://lore.kernel.org/qemu-devel/20250318130219.1799170-16-zhao1.liu@intel.com/
> Speaking of size_of, I noticed that Rust provides both
> core::mem::size_of and std::mem::size_of, and in rust/ at
> the moment we have uses of both. What's the difference?
They're the same (a simple proof of this is that the "source" option of
the std::mem page [2] points to the core::mem repo). `core` is
self-contained without OS dependency, and `std` is the superset of `core`
with extra OS dependency. And there's a previous cleanup to consolidate
`std::ptr` (commit c48700e86d, "rust: prefer importing std::ptr over
core::ptr"). So I think we can prefer std::mem as well.
[2]: https://doc.rust-lang.org/std/mem/index.html
Regards,
Zhao
prev parent reply other threads:[~2025-03-21 4:12 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-20 13:32 [PATCH 0/3] rust: Fix PL011State size mismatch assert Peter Maydell
2025-03-20 13:32 ` [PATCH 1/3] rust: assertions: add static_assert Peter Maydell
2025-03-20 14:03 ` Philippe Mathieu-Daudé
2025-03-20 15:20 ` Zhao Liu
2025-03-20 13:32 ` [PATCH 2/3] hw/char/pl011: Pad PL011State struct to same size as Rust impl Peter Maydell
2025-03-20 15:28 ` Zhao Liu
2025-03-20 13:32 ` [PATCH 3/3] rust: pl011: Check size of state struct at compile time Peter Maydell
2025-03-20 15:45 ` Zhao Liu
2025-03-20 16:02 ` Peter Maydell
2025-03-21 4:31 ` Zhao Liu [this message]
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=Z9zrvF7qRoykMupa@intel.com \
--to=zhao1.liu@intel.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu-rust@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).