All of lore.kernel.org
 help / color / mirror / Atom feed
From: Baoquan He <bhe@redhat.com>
To: kexec@lists.infradead.org
Subject: [RESEND PATCH v3 1/3] kexec: clean up arch_kexec_kernel_verify_sig
Date: Fri, 18 Mar 2022 16:54:01 +0800	[thread overview]
Message-ID: <YjRIqdAH2nV+DRgW@MiWiFi-R3L-srv> (raw)
In-Reply-To: <20220318071824.yq3idr5eoogvtslb@Rk>

On 03/18/22 at 03:18pm, Coiby Xu wrote:
> On Fri, Mar 18, 2022 at 11:27:09AM +0800, Baoquan He wrote:
> > On 03/18/22 at 10:48am, Coiby Xu wrote:
> > > On Thu, Mar 17, 2022 at 08:45:35PM +0800, Baoquan He wrote:
> > > > On 03/04/22 at 10:03am, Coiby Xu wrote:
> > > > > From: Coiby Xu <coxu@redhat.com>
> > > > >
> > > > > commit 9ec4ecef0af7790551109283ca039a7c52de343c ("kexec_file,x86,
> > > > > powerpc: factor out kexec_file_ops functions" allows implementing
> > > > > the arch-specific implementation of kernel image verification
> > > > > in kexec_file_ops->verify_sig. Currently, there is no arch-specific
> > > > > implementation of arch_kexec_kernel_verify_sig. So clean it up.
> > > >
> > > > This is a nice cleanup, while the log may need to be improved. You
> > > > should run ./scripts/checkpatch.pl on your patch before sending out.
> > > > When we refer to a commit in log, please refer to
> > > > Documentation/process/submitting-patches.rst.
> > > 
> > > Thanks for the reminder! I've used git pre-commit hook to run
> > > scripts/checkpatch.pl automatically but obviously this hook doesn't
> > > apply to "git rebase --continue" and currently this no git hook that
> > > for this situation. I'll use the following trick [1] to avoid this
> > > mistake in the future,
> > >  $ git rebase -i HEAD~3 --reschedule-failed-exec --exec "git show | perl ./scripts/checkpatch.pl"
> > 
> > Sorry, Coiby. It could be late yesterday so I was dizzy when writing
> > down the comment, I didn't make my concern clear. What I meant is
> > the referenced commit in log should be taken in a standard format.
> > Abstracted one paragraph of Documentation/process/submitting-patches.rst
> > here. We usually take the first 12 characters of the commit SHA-1 ID
> > in log, but not the whole of them.
> > 
> > =====
> > If you want to refer to a specific commit, don't just refer to the
> > SHA-1 ID of the commit. Please also include the oneline summary of
> > the commit, to make it easier for reviewers to know what it is about.
> > Example::
> > 
> >        Commit e21d2170f36602ae2708 ("video: remove unnecessary
> >        platform_set_drvdata()") removed the unnecessary
> >        platform_set_drvdata(), but left the variable "dev" unused,
> >        delete it.
> > =====
> > 
> > And the right parenthesis enclousing the commit subject is missing.
> 
> Thanks for the detailed explanation! Your message has got across to me
> successfully:) I have ran scripts/checkpatch.pl manually after seeing your
> first reply and checkpatch.pl reported the exact same issues as explained
> by you today. My approach of avoiding making mistakes on format is to run
> checkpatch.pl automatically in the git precommit hook so I don't need to
> remember the details about format. I had expected the git precommit hook
> could help me find the issues pointed out by you but obviously it failed.
> So I tried to find out what's wrong. I think the format issues were
> introduced when doing rebase to improve the old version and the precommit
> hook wasn't triggered in this case. Another thing I still missed is I used
> "git diff --cached | scripts/checkpatch.pl" in the pre-commit hook which
> obviously won't check the format issue in the commit message (it only
> check the format issue in the code). With the two problems resolved, I
> shall not make format mistakes in the future:)
> 
> Btw, checkpatch.pl seems to requires referring to a specific commit on
> the same line,
> 
>     ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 9ec4ecef0af7 ("kexec_file,x86,powerpc: factor out kexec_file_ops functions")'
>     #6:     commit 9ec4ecef0af7 ("kexec_file,x86, powerpc: factor out
> kexec_file_ops
>     functions") allows implementing the arch-specific implementation of kernel
>     total: 1 errors, 0 warnings, 61 lines checked
>     NOTE: For some of the reported defects, checkpatch may be able to
>           mechanically convert to the typical style using --fix or --fix-inplace.
>     "[PATCH] kexec: clean up arch_kexec_kernel_verify_sig" has style problems, please review.
>     NOTE: If any of the errors are false positives, please report
>           them to the maintainer, see CHECKPATCH in MAINTAINERS.
> 
> Is this a false positive?

No, it's not. Youp probably copied the commit subject and modified it.
Please copy below two lines into your patch to replace and try again.

commit 9ec4ecef0af7 ("kexec_file,x86,powerpc: factor out kexec_file_ops
functions")

> 
> > 
> > > 
> > > [1] https://stackoverflow.com/a/70568833/1203522
> > > 
> > > 
> > > --
> > > Best regards,
> > > Coiby
> > > 
> > 
> 
> -- 
> Best regards,
> Coiby
> 



WARNING: multiple messages have this Message-ID (diff)
From: Baoquan He <bhe@redhat.com>
To: Coiby Xu <coxu@redhat.com>
Cc: Coiby Xu <coiby.xu@gmail.com>,
	kexec@lists.infradead.org, linux-arm-kernel@lists.infradead.org,
	Dave Young <dyoung@redhat.com>, Will Deacon <will@kernel.org>,
	"Eric W . Biederman" <ebiederm@xmission.com>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [RESEND PATCH v3 1/3] kexec: clean up arch_kexec_kernel_verify_sig
Date: Fri, 18 Mar 2022 16:54:01 +0800	[thread overview]
Message-ID: <YjRIqdAH2nV+DRgW@MiWiFi-R3L-srv> (raw)
In-Reply-To: <20220318071824.yq3idr5eoogvtslb@Rk>

On 03/18/22 at 03:18pm, Coiby Xu wrote:
> On Fri, Mar 18, 2022 at 11:27:09AM +0800, Baoquan He wrote:
> > On 03/18/22 at 10:48am, Coiby Xu wrote:
> > > On Thu, Mar 17, 2022 at 08:45:35PM +0800, Baoquan He wrote:
> > > > On 03/04/22 at 10:03am, Coiby Xu wrote:
> > > > > From: Coiby Xu <coxu@redhat.com>
> > > > >
> > > > > commit 9ec4ecef0af7790551109283ca039a7c52de343c ("kexec_file,x86,
> > > > > powerpc: factor out kexec_file_ops functions" allows implementing
> > > > > the arch-specific implementation of kernel image verification
> > > > > in kexec_file_ops->verify_sig. Currently, there is no arch-specific
> > > > > implementation of arch_kexec_kernel_verify_sig. So clean it up.
> > > >
> > > > This is a nice cleanup, while the log may need to be improved. You
> > > > should run ./scripts/checkpatch.pl on your patch before sending out.
> > > > When we refer to a commit in log, please refer to
> > > > Documentation/process/submitting-patches.rst.
> > > 
> > > Thanks for the reminder! I've used git pre-commit hook to run
> > > scripts/checkpatch.pl automatically but obviously this hook doesn't
> > > apply to "git rebase --continue" and currently this no git hook that
> > > for this situation. I'll use the following trick [1] to avoid this
> > > mistake in the future,
> > >  $ git rebase -i HEAD~3 --reschedule-failed-exec --exec "git show | perl ./scripts/checkpatch.pl"
> > 
> > Sorry, Coiby. It could be late yesterday so I was dizzy when writing
> > down the comment, I didn't make my concern clear. What I meant is
> > the referenced commit in log should be taken in a standard format.
> > Abstracted one paragraph of Documentation/process/submitting-patches.rst
> > here. We usually take the first 12 characters of the commit SHA-1 ID
> > in log, but not the whole of them.
> > 
> > =====
> > If you want to refer to a specific commit, don't just refer to the
> > SHA-1 ID of the commit. Please also include the oneline summary of
> > the commit, to make it easier for reviewers to know what it is about.
> > Example::
> > 
> >        Commit e21d2170f36602ae2708 ("video: remove unnecessary
> >        platform_set_drvdata()") removed the unnecessary
> >        platform_set_drvdata(), but left the variable "dev" unused,
> >        delete it.
> > =====
> > 
> > And the right parenthesis enclousing the commit subject is missing.
> 
> Thanks for the detailed explanation! Your message has got across to me
> successfully:) I have ran scripts/checkpatch.pl manually after seeing your
> first reply and checkpatch.pl reported the exact same issues as explained
> by you today. My approach of avoiding making mistakes on format is to run
> checkpatch.pl automatically in the git precommit hook so I don't need to
> remember the details about format. I had expected the git precommit hook
> could help me find the issues pointed out by you but obviously it failed.
> So I tried to find out what's wrong. I think the format issues were
> introduced when doing rebase to improve the old version and the precommit
> hook wasn't triggered in this case. Another thing I still missed is I used
> "git diff --cached | scripts/checkpatch.pl" in the pre-commit hook which
> obviously won't check the format issue in the commit message (it only
> check the format issue in the code). With the two problems resolved, I
> shall not make format mistakes in the future:)
> 
> Btw, checkpatch.pl seems to requires referring to a specific commit on
> the same line,
> 
>     ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 9ec4ecef0af7 ("kexec_file,x86,powerpc: factor out kexec_file_ops functions")'
>     #6:     commit 9ec4ecef0af7 ("kexec_file,x86, powerpc: factor out
> kexec_file_ops
>     functions") allows implementing the arch-specific implementation of kernel
>     total: 1 errors, 0 warnings, 61 lines checked
>     NOTE: For some of the reported defects, checkpatch may be able to
>           mechanically convert to the typical style using --fix or --fix-inplace.
>     "[PATCH] kexec: clean up arch_kexec_kernel_verify_sig" has style problems, please review.
>     NOTE: If any of the errors are false positives, please report
>           them to the maintainer, see CHECKPATCH in MAINTAINERS.
> 
> Is this a false positive?

No, it's not. Youp probably copied the commit subject and modified it.
Please copy below two lines into your patch to replace and try again.

commit 9ec4ecef0af7 ("kexec_file,x86,powerpc: factor out kexec_file_ops
functions")

> 
> > 
> > > 
> > > [1] https://stackoverflow.com/a/70568833/1203522
> > > 
> > > 
> > > --
> > > Best regards,
> > > Coiby
> > > 
> > 
> 
> -- 
> Best regards,
> Coiby
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Baoquan He <bhe@redhat.com>
To: Coiby Xu <coxu@redhat.com>
Cc: Coiby Xu <coiby.xu@gmail.com>,
	kexec@lists.infradead.org, linux-arm-kernel@lists.infradead.org,
	Dave Young <dyoung@redhat.com>, Will Deacon <will@kernel.org>,
	"Eric W . Biederman" <ebiederm@xmission.com>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [RESEND PATCH v3 1/3] kexec: clean up arch_kexec_kernel_verify_sig
Date: Fri, 18 Mar 2022 16:54:01 +0800	[thread overview]
Message-ID: <YjRIqdAH2nV+DRgW@MiWiFi-R3L-srv> (raw)
In-Reply-To: <20220318071824.yq3idr5eoogvtslb@Rk>

On 03/18/22 at 03:18pm, Coiby Xu wrote:
> On Fri, Mar 18, 2022 at 11:27:09AM +0800, Baoquan He wrote:
> > On 03/18/22 at 10:48am, Coiby Xu wrote:
> > > On Thu, Mar 17, 2022 at 08:45:35PM +0800, Baoquan He wrote:
> > > > On 03/04/22 at 10:03am, Coiby Xu wrote:
> > > > > From: Coiby Xu <coxu@redhat.com>
> > > > >
> > > > > commit 9ec4ecef0af7790551109283ca039a7c52de343c ("kexec_file,x86,
> > > > > powerpc: factor out kexec_file_ops functions" allows implementing
> > > > > the arch-specific implementation of kernel image verification
> > > > > in kexec_file_ops->verify_sig. Currently, there is no arch-specific
> > > > > implementation of arch_kexec_kernel_verify_sig. So clean it up.
> > > >
> > > > This is a nice cleanup, while the log may need to be improved. You
> > > > should run ./scripts/checkpatch.pl on your patch before sending out.
> > > > When we refer to a commit in log, please refer to
> > > > Documentation/process/submitting-patches.rst.
> > > 
> > > Thanks for the reminder! I've used git pre-commit hook to run
> > > scripts/checkpatch.pl automatically but obviously this hook doesn't
> > > apply to "git rebase --continue" and currently this no git hook that
> > > for this situation. I'll use the following trick [1] to avoid this
> > > mistake in the future,
> > >  $ git rebase -i HEAD~3 --reschedule-failed-exec --exec "git show | perl ./scripts/checkpatch.pl"
> > 
> > Sorry, Coiby. It could be late yesterday so I was dizzy when writing
> > down the comment, I didn't make my concern clear. What I meant is
> > the referenced commit in log should be taken in a standard format.
> > Abstracted one paragraph of Documentation/process/submitting-patches.rst
> > here. We usually take the first 12 characters of the commit SHA-1 ID
> > in log, but not the whole of them.
> > 
> > =====
> > If you want to refer to a specific commit, don't just refer to the
> > SHA-1 ID of the commit. Please also include the oneline summary of
> > the commit, to make it easier for reviewers to know what it is about.
> > Example::
> > 
> >        Commit e21d2170f36602ae2708 ("video: remove unnecessary
> >        platform_set_drvdata()") removed the unnecessary
> >        platform_set_drvdata(), but left the variable "dev" unused,
> >        delete it.
> > =====
> > 
> > And the right parenthesis enclousing the commit subject is missing.
> 
> Thanks for the detailed explanation! Your message has got across to me
> successfully:) I have ran scripts/checkpatch.pl manually after seeing your
> first reply and checkpatch.pl reported the exact same issues as explained
> by you today. My approach of avoiding making mistakes on format is to run
> checkpatch.pl automatically in the git precommit hook so I don't need to
> remember the details about format. I had expected the git precommit hook
> could help me find the issues pointed out by you but obviously it failed.
> So I tried to find out what's wrong. I think the format issues were
> introduced when doing rebase to improve the old version and the precommit
> hook wasn't triggered in this case. Another thing I still missed is I used
> "git diff --cached | scripts/checkpatch.pl" in the pre-commit hook which
> obviously won't check the format issue in the commit message (it only
> check the format issue in the code). With the two problems resolved, I
> shall not make format mistakes in the future:)
> 
> Btw, checkpatch.pl seems to requires referring to a specific commit on
> the same line,
> 
>     ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 9ec4ecef0af7 ("kexec_file,x86,powerpc: factor out kexec_file_ops functions")'
>     #6:     commit 9ec4ecef0af7 ("kexec_file,x86, powerpc: factor out
> kexec_file_ops
>     functions") allows implementing the arch-specific implementation of kernel
>     total: 1 errors, 0 warnings, 61 lines checked
>     NOTE: For some of the reported defects, checkpatch may be able to
>           mechanically convert to the typical style using --fix or --fix-inplace.
>     "[PATCH] kexec: clean up arch_kexec_kernel_verify_sig" has style problems, please review.
>     NOTE: If any of the errors are false positives, please report
>           them to the maintainer, see CHECKPATCH in MAINTAINERS.
> 
> Is this a false positive?

No, it's not. Youp probably copied the commit subject and modified it.
Please copy below two lines into your patch to replace and try again.

commit 9ec4ecef0af7 ("kexec_file,x86,powerpc: factor out kexec_file_ops
functions")

> 
> > 
> > > 
> > > [1] https://stackoverflow.com/a/70568833/1203522
> > > 
> > > 
> > > --
> > > Best regards,
> > > Coiby
> > > 
> > 
> 
> -- 
> Best regards,
> Coiby
> 


  reply	other threads:[~2022-03-18  8:54 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-04  2:03 [RESEND PATCH v3 0/3] use more system keyrings to verify arm64 kdump kernel image signature Coiby Xu
2022-03-04  2:03 ` Coiby Xu
2022-03-04  2:03 ` [RESEND PATCH v3 1/3] kexec: clean up arch_kexec_kernel_verify_sig Coiby Xu
2022-03-04  2:03   ` Coiby Xu
2022-03-04  2:03   ` Coiby Xu
2022-03-17 12:45   ` Baoquan He
2022-03-17 12:45     ` Baoquan He
2022-03-17 12:45     ` Baoquan He
2022-03-18  2:48     ` Coiby Xu
2022-03-18  2:48       ` Coiby Xu
2022-03-18  2:48       ` Coiby Xu
2022-03-18  3:27       ` Baoquan He
2022-03-18  3:27         ` Baoquan He
2022-03-18  3:27         ` Baoquan He
2022-03-18  7:18         ` Coiby Xu
2022-03-18  7:18           ` Coiby Xu
2022-03-18  7:18           ` Coiby Xu
2022-03-18  8:54           ` Baoquan He [this message]
2022-03-18  8:54             ` Baoquan He
2022-03-18  8:54             ` Baoquan He
2022-03-18  9:43             ` Coiby Xu
2022-03-18  9:43               ` Coiby Xu
2022-03-18  9:43               ` Coiby Xu
2022-03-04  2:03 ` [RESEND PATCH v3 2/3] kexec, KEYS: make the code in bzImage64_verify_sig generic Coiby Xu
2022-03-04  2:03   ` Coiby Xu
2022-03-04  2:03   ` Coiby Xu
2022-03-04  2:03 ` [RESEND PATCH v3 3/3] arm64: kexec_file: use more system keyrings to verify kernel image signature Coiby Xu
2022-03-04  2:03   ` Coiby Xu
2022-03-04  2:03   ` Coiby Xu

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=YjRIqdAH2nV+DRgW@MiWiFi-R3L-srv \
    --to=bhe@redhat.com \
    --cc=kexec@lists.infradead.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.