linux-kernel - Re: [PATCH 1/1] Use new `--output-format=doctest` rustdoc command line flag to improve doctest handling.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAAOQCfSZ=HGWjeN-3fotNJpW35iqzm0A_3rxs=BdpULmLyrs2w@mail.gmail.com>
Date: Fri, 28 Feb 2025 17:55:36 +0100
From: Guillaume Gomez <guillaume1.gomez@...il.com>
To: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Cc: ojeda@...nel.org, rust-for-linux@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] Use new `--output-format=doctest` rustdoc command
 line flag to improve doctest handling.

> That would still force us to have all the "hardcoded knowledge" about
> the `rustdoc` output (apart from the JSON schema), right?

That reduces it, but doesn't solve the problem completely. The second part
is more tricky.

> i.e. my idea was to try to see if we could avoid matching on "strings"
> as much as possible, and trying to have enough metadata (properly
> encoded in the JSON somehow), so that we need to avoid searching for
> e.g. `main()` etc.; and instead generate everything else needed on our
> side, customized for the kernel case.

I think in this case, the only issue is that we're calling `_inner().unwrap()`
right? So we could go around this issue by not generating this code
directly in the doctest output and instead add a new field:

* return_result_fn_name: Contains the name of the wrapping function
  returning the `Result` type. If this field is present, it means that the
  doctest is returning a `Result` and that we need to add in the code:
  `assert!({return_result_fn_name}.is_ok())`.

But even that is tricky because it's part of another block (`{}`) so the users
would need to add an extra `}` after the call. Anyway, I think it'll need to
be discussed more in details in another discussion on how to best
implement it for both sides.

Le ven. 28 févr. 2025 à 17:48, Miguel Ojeda
<miguel.ojeda.sandonis@...il.com> a écrit :
>
> On Fri, Feb 28, 2025 at 5:32 PM Guillaume Gomez
> <guillaume1.gomez@...il.com> wrote:
> >
> > I'll definitely need some help here. I'm not sure current stable already
> > has this change so until then, we'll need a beta/nightly version to run
> > these tests.
>
> Yeah, we will need to wait until the "final" version of the flag is in
> a stable version (the flag would not need to be "stable" itself, nor
> the compiler released, but yeah, we need to know the version).
>
> > I opened https://github.com/rust-lang/rust/pull/137807 to resolve
> > this problem.
>
> Thanks!
>
> That would still force us to have all the "hardcoded knowledge" about
> the `rustdoc` output (apart from the JSON schema), right?
>
> i.e. my idea was to try to see if we could avoid matching on "strings"
> as much as possible, and trying to have enough metadata (properly
> encoded in the JSON somehow), so that we need to avoid searching for
> e.g. `main()` etc.; and instead generate everything else needed on our
> side, customized for the kernel case.
>
> > I don't think `expect` would work in any of the cases in this file. What I suggest
> > is to add methods on `JsonValue` in a future patch which would allow to reduce
> > code in this file (and call `expect` too).
>
> Yeah, sorry, when I saw the `Some(...) ... else panic!` I replied too
> quickly -- in this case, I don't think it matters to have a custom
> error for the "wrong JSON type" case as we discussed offline since
> nobody should be seeing the error to begin with, so it is fine.
>
> Cheers,
> Miguel