linux-kernel - Re: [PATCH v3 1/1] rust: device: rename "Device::from

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAH5fLgjMGwBNYkOEbiSLzQ5+G0gTz+gbvsRSoRsRizAETLpLeQ@mail.gmail.com>
Date: Mon, 30 Sep 2024 17:55:10 +0200
From: Alice Ryhl <aliceryhl@...gle.com>
To: Guilherme Giácomo Simões <trintaeoitogc@...il.com>
Cc: Greg KH <gregkh@...uxfoundation.org>, rafael@...nel.org, ojeda@...nel.org, 
	alex.gaynor@...il.com, boqun.feng@...il.com, gary@...yguo.net, 
	bjorn3_gh@...tonmail.com, benno.lossin@...ton.me, mcgrof@...nel.org, 
	russ.weight@...ux.dev, dakr@...hat.com, a.hindborg@...nel.org, 
	rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/1] rust: device: rename "Device::from_raw()"

On Mon, Sep 30, 2024 at 4:58 PM Guilherme Giácomo Simões
<trintaeoitogc@...il.com> wrote:
>
> Greg KH <gregkh@...uxfoundation.org> writes:
> >
> > On Mon, Sep 30, 2024 at 11:43:27AM -0300, Guilherme Giacomo Simoes wrote:
> > > This function increments the refcount by a call to
> > > "bindings::get_device(ptr)". This can be confused because, the function
> > > Arch::from_raw() from standard library, don't increments the refcount.
> > > Hence, rename "Device::from_raw()" to avoid confusion with other
> > > "from_raw" semantics.
> > >
> > > Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@...il.com>
> > > ---
> > >  rust/kernel/device.rs   | 2 +-
> > >  rust/kernel/firmware.rs | 2 +-
> > >  2 files changed, 2 insertions(+), 2 deletions(-)
> > >
> >
> > Hi,
> >
> > This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
> > a patch that has triggered this response.  He used to manually respond
> > to these common problems, but in order to save his sanity (he kept
> > writing the same thing over and over, yet to different people), I was
> > created.  Hopefully you will not take offence and will fix the problem
> > in your patch and resubmit it so that it can be accepted into the Linux
> > kernel tree.
> >
> > You are receiving this message because of the following common error(s)
> > as indicated below:
> >
> > - This looks like a new version of a previously submitted patch, but you
> >   did not list below the --- line any changes from the previous version.
> >   Please read the section entitled "The canonical patch format" in the
> >   kernel file, Documentation/process/submitting-patches.rst for what
> >   needs to be done here to properly describe this.
> >
> > If you wish to discuss this problem further, or you have questions about
> > how to resolve this issue, please feel free to respond to this email and
> > Greg will reply once he has dug out from the pending patches received
> > from other developers.
> >
> > thanks,
> >
> > greg k-h's patch email bot
>
> Yeah, I was think that only in 0/1 I should explain the changes ..
> I'm was wrong.   I'll put the changelog in 1/1 too.

You can use one of my patches as an example. E.g.:
https://lore.kernel.org/all/20240930-static-mutex-v4-1-c59555413127@google.com/

Here, the commit message itself has:
1. Motivation for why we should add global lock support. (To replace a
hack I had to use in the Binder driver.)
2. Explanation for why I implemented it in a certain way. (Why
separate initialization step?)

Then, below the --- line and not part of the commit message, I have:
1. Information about which patches it depends on.
2. A changelog and links to previous versions.

Anything below the --- line will not be part of the commit history
when your change is merged. So you should think about what people
would want to see when they look at your patch in the commit history.
They care about why the change was made, and why it was implemented
that way. What other things need to be merged first are not relevant
to people who see the final version after it has been merged.

Similarly, the changelog is important for reviewers so they can
compare with the previous version, but for people who just see the
final version, they don't care about which bugs you had in previous
versions of the patch. Of course, if you change the implementation
approach, then they might care about why you chose that approach over
some other approach, but that explanation should be in the commit
message (and the changelog should just say you changed the approach).

Alice