Skip to content

Tracking Issue for NUL-terminated file names with #[track_caller] #141727

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
1 of 4 tasks
Darksonn opened this issue May 29, 2025 · 3 comments
Open
1 of 4 tasks

Tracking Issue for NUL-terminated file names with #[track_caller] #141727

Darksonn opened this issue May 29, 2025 · 3 comments
Labels
A-panic Area: Panicking machinery A-rust-for-linux Relevant for the Rust-for-Linux project C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC S-tracking-impl-incomplete Status: The implementation is incomplete. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@Darksonn
Copy link
Contributor

Darksonn commented May 29, 2025

Feature gate: #![feature(location_file_nul)]

This is a tracking issue for Location::file_with_nul.

This feature allows you to obtain NUL-terminated file names from core::panic::Location when using #[track_caller]. This allows for better error messages in projects performing interop with C/C++.

Public API

// core::panic

pub struct Location;

impl Location {
    pub const fn file_with_nul(&self) -> &CStr;
}

Steps / History

(Remember to update the S-tracking-* label when checking boxes.)

Related PRs:

Unresolved Questions

It's still unresolved whether the above API is preferred over returning a raw pointer:

// core::panic

pub struct Location;

impl Location {
    pub const fn file_ptr(&self) -> *const c_char;
}

Although this solution is less safe to use, it has the advantage that if we change Location to not store the length, then you can call Location::file_ptr without having to invoke strlen to construct a &CStr.

Footnotes

  1. https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html

@Darksonn Darksonn added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs Relevant to the library team, which will review and decide on the PR/issue. A-panic Area: Panicking machinery A-rust-for-linux Relevant for the Rust-for-Linux project labels May 29, 2025
@Darksonn Darksonn added the S-tracking-impl-incomplete Status: The implementation is incomplete. label May 29, 2025
@bonzini
Copy link

bonzini commented May 29, 2025

As a possible user of the feature, returning *const c_char would be just fine. I have no use for the stored file other than at the Rust->C boundary, and would use file.as_ptr() there.

The only minor nuisance of storing a *const instead of a CStr is that it blocks automatic implementation of Send and Sync.

@bjorn3
Copy link
Member

bjorn3 commented May 29, 2025

We need to store the length anyway for Location::file(), so returning &CStr from Location::file_with_nul shouldn't increase the size of Location at all.

@Darksonn
Copy link
Contributor Author

@bjorn3 That's not true. Check out #117431 which used strlen every time Location::file() is called to compute the length in exchange for reducing the size of Location.

pbo-linaro pushed a commit to pbo-linaro/qemu-ci that referenced this issue May 30, 2025
https://lore.kernel.org/qemu-devel/20250530080307.2055502-1-pbonzini@redhat.com

---

From: Paolo Bonzini <pbonzini@redhat.com>
To: qemu-devel@nongnu.org
Cc: qemu-rust@nongnu.org
Subject: [PATCH v2 00/14] rust: bindings for Error
Date: Fri, 30 May 2025 10:02:52 +0200
Message-ID: <20250530080307.2055502-1-pbonzini@redhat.com>
X-Mailer: git-send-email 2.49.0
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Received-SPF: pass client-ip=170.10.133.124; envelope-from=pbonzini@redhat.com;
 helo=us-smtp-delivery-124.mimecast.com
X-Spam_score_int: -49
X-Spam_score: -5.0
X-Spam_bar: -----
X-Spam_report: (-5.0 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-2.902,
 DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1,
 RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H5=0.001, RCVD_IN_MSPIKE_WL=0.001,
 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001,
 SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no
X-Spam_action: no action
X-BeenThere: qemu-devel@nongnu.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
 <mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <https://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
 <mailto:qemu-devel-request@nongnu.org?subject=subscribe>
Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org
Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org

As explained for v1, the impetus for this series is to remove BqlCell<>
from HPETState::num_timers.  However, it's also an important step for QAPI:
error propagation is pretty central for example to QMP, and the series
is also a first example of two-way conversion between C and native-Rust
structs (i.e. not using bindgen-generated structs or their opaque wrappers).

As an aside, support for NUL-terminated file is now scheduled for
inclusion in Rust as "panic::Location::file_with_nul()", but it will be
quite a while before QEMU can use it.  For more information, see
rust-lang/rust#141727.

Paolo

v1->v2:
- patch "rust: make declaration of dependent crates more consistent" merged
- change dependency name for anyhow to anyhow-1-rs
- update scripts/archive-source.sh and scripts/make-release [Zhao]
- update foreign to 0.3.1 instead of 0.2.0
- use %.*s to print non-NUL-terminated err->src [Markus]
- make err->src_len an int instead of a size_t [Markus]
- add doc comment for error module
- remove #[derive(Default)] for Error [Markus]
- rewrite ok_or_propagate in functional style [Markus]
- clarify "validity" of Error** [Markus]
- clarify that err_or_unit/err_or_else free the Error* [Markus]

new patches:
- hpet: adjust VMState for consistency with Rust version [Zhao]
- rust: qemu-api: add tests for Error bindings
- docs: update Rust module status

Paolo Bonzini (13):
  subprojects: add the anyhow crate
  subprojects: add the foreign crate
  util/error: expose Error definition to Rust code
  util/error: allow non-NUL-terminated err->src
  util/error: make func optional
  rust: qemu-api: add bindings to Error
  rust: qemu-api: add tests for Error bindings
  rust: qdev: support returning errors from realize
  rust/hpet: change type of num_timers to usize
  hpet: adjust VMState for consistency with Rust version
  hpet: return errors from realize if properties are incorrect
  rust/hpet: return errors from realize if properties are incorrect
  docs: update Rust module status

Zhao Liu (1):
  rust/hpet: Drop BqlCell wrapper for num_timers

 docs/devel/rust.rst                           |   2 +-
 include/qapi/error-internal.h                 |  27 ++
 rust/wrapper.h                                |   1 +
 hw/timer/hpet.c                               |  21 +-
 util/error.c                                  |  20 +-
 rust/Cargo.lock                               |  17 +
 rust/Cargo.toml                               |   1 +
 rust/hw/char/pl011/src/device.rs              |   5 +-
 rust/hw/timer/hpet/src/device.rs              |  62 ++-
 rust/hw/timer/hpet/src/fw_cfg.rs              |   7 +-
 rust/meson.build                              |   4 +
 rust/qemu-api/Cargo.toml                      |   2 +
 rust/qemu-api/meson.build                     |   3 +-
 rust/qemu-api/src/error.rs                    | 403 ++++++++++++++++++
 rust/qemu-api/src/lib.rs                      |   3 +
 rust/qemu-api/src/qdev.rs                     |  10 +-
 scripts/archive-source.sh                     |   5 +-
 scripts/make-release                          |   5 +-
 subprojects/.gitignore                        |   2 +
 subprojects/anyhow-1-rs.wrap                  |   7 +
 subprojects/foreign-0.3-rs.wrap               |   7 +
 .../packagefiles/anyhow-1.0-rs/meson.build    |  33 ++
 .../packagefiles/foreign-0.3-rs/meson.build   |  26 ++
 23 files changed, 602 insertions(+), 71 deletions(-)
 create mode 100644 include/qapi/error-internal.h
 create mode 100644 rust/qemu-api/src/error.rs
 create mode 100644 subprojects/anyhow-1-rs.wrap
 create mode 100644 subprojects/foreign-0.3-rs.wrap
 create mode 100644 subprojects/packagefiles/anyhow-1.0-rs/meson.build
 create mode 100644 subprojects/packagefiles/foreign-0.3-rs/meson.build

--
2.49.0

Signed-off-by: GitHub Actions Bot <bot@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-panic Area: Panicking machinery A-rust-for-linux Relevant for the Rust-for-Linux project C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC S-tracking-impl-incomplete Status: The implementation is incomplete. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants