Re: [PATCH] rust: log: implement io::Write

[Manos Pitsidianakis <manos.pitsidianakis@linaro.org>]

On Tue, Jun 24, 2025 at 10:00 AM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> >  /// A macro to log messages conditionally based on a provided mask.
> > @@ -24,6 +96,8 @@ pub enum Log {
> >  /// log level and, if so, formats and logs the message. It is the Rust
> >  /// counterpart of the `qemu_log_mask()` macro in the C implementation.
> >  ///
> > +/// Errors from writing to the log are ignored.
> > +///
> >  /// # Parameters
> >  ///
> >  /// - `$mask`: A log level mask. This should be a variant of the `Log` enum.
> > @@ -62,12 +136,9 @@ macro_rules! log_mask_ln {
> >          if unsafe {
> >              (::qemu_api::bindings::qemu_loglevel & ($mask as std::os::raw::c_int)) != 0
> >          } {
> > -            let formatted_string = format!("{}\n", format_args!($fmt $($args)*));
> > -            let c_string = std::ffi::CString::new(formatted_string).unwrap();
> > -
> > -            unsafe {
> > -                ::qemu_api::bindings::qemu_log(c_string.as_ptr());
> > -            }
> > +            #[allow(unused_must_use)]
>
> I found this doesn't work :-( :
>
> error: unused `Result` that must be used
>    --> ../rust/hw/char/pl011/src/device.rs:281:21
>     |
> 281 |                     log_mask_ln!(Log::Unimp, "pl011: DMA not implemented");
>     |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>     |
>     = note: this `Result` may be an `Err` variant, which should be handled
>     = note: `-D unused-must-use` implied by `-D warnings`
>     = help: to override `-D warnings` add `#[allow(unused_must_use)]`
>     = note: this error originates in the macro `log_mask_ln` (in Nightly builds, run with -Z macro-backtrace for more info)
>
> I understand meson sets `-D warings` so that `allow` can't work...
>
> What about just ignoring the return value? Afterall pl011 doesn't care
> the returned value.
>
> @@ -136,8 +137,7 @@ macro_rules! log_mask_ln {
>          if unsafe {
>              (::qemu_api::bindings::qemu_loglevel & ($mask as std::os::raw::c_int)) != 0
>          } {
> -            #[allow(unused_must_use)]
> -            ::qemu_api::log::LogGuard::log_fmt(
> +            let _ = ::qemu_api::log::LogGuard::log_fmt(
>                  format_args!("{}\n", format_args!($fmt $($args)*)));
>          }
>      }};


Just `_ = ::qemu_api::log::LogGuard::log_fmt(...);` is sufficient, no
`let` needed.

Paolo: I haven't tested it to see the warning Zhao did (busy
lately...), but LGTM, as well. With that fixed,

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>