linux-kernel - RE: [PATCH] drivers: hv: channel: fixed a tab having spaces before hv: connection: fixed required space for "=" sign before.

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: <MWHPR21MB1593FF42E1BD760509942F9ED7849@MWHPR21MB1593.namprd21.prod.outlook.com>
Date:   Fri, 19 Feb 2021 16:32:05 +0000
From:   Michael Kelley <mikelley@...rosoft.com>
To:     Vasanth <vasanth3g@...il.com>, KY Srinivasan <kys@...rosoft.com>
CC:     Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        "wei.liu@...nel.org" <wei.liu@...nel.org>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] drivers: hv: channel: fixed a tab having spaces before
        hv: connection: fixed required space for "=" sign before.

From: Vasanth <vasanth3g@...il.com> Sent: Thursday, February 18, 2021 3:15 AM
> 
> Fixed checkpatch warning: Tab space before having normal space.
> 
> Fixed checkpatch error: Required space for "=" sign before.
> 

I think this commit message is appropriate.  But I would still suggest
simplifying the "Subject" line.  The Subject line should ideally be no
more than about 70 characters, and just needs to summarize.  So
go with something like:

[PATCH] drivers: hv: Fix whitespace errors

Or:

[PATCH] drivers: hv: Fix whitespace errors from checkpatch

The "drivers: hv:" portion just identifies the general area of the
kernel that is affected by the patch, and doesn't need to identify
every file that is touched.  And when someone is scanning through a
list of commits and looking at the commit titles, the words "Fix
whitespace errors" will be a perfect summary.  The person will know
that the commit probably didn't fix whatever bug they are chasing
down. :-)

One other thing:  When sending an updated version of a patch,
it is customary to add a version number, even if the change is just
to the Subject or the commit message.  So your Subject could
have been:

[PATCH v2] drivers: hv: Fix whitespace errors

When you send your next version, calling it "v3" would be
OK.

> Signed-off-by: Vasanth <vasanth3g@...il.com>
> ---

The text below the "---" does *not* get included in the
official commit message when a patch is accepted.  Submitters
usually use this area for a bit of a revision history when a patch
is revised and a new version produced.  So your revision history
might look like this:

Changes in v2:
*  Added commit message
*  Revised Subject

For a small patch like yours, such a revision history isn't
super important, but it is still a good practice.  The revision
history is much more important for a large patch.  Somebody
might spend 30 minutes carefully reviewing v3 of a large patch,
and provide comments.  When the creator sends v4 of the patch,
the reviewer would really like to know what changed, so he
doesn't necessarily have to review the entire patch again
from scratch.

I would suggest looking through other patches submitted to
the Linux Kernel Mailing List.  You'll see examples of adding
"v2", "v3", etc. as well as examples of the revision history.

Michael

>  drivers/hv/channel.c    | 2 +-
>  drivers/hv/connection.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index 6fb0c76bfbf8..587234065e37 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -385,7 +385,7 @@ static int create_gpadl_header(enum hv_gpadl_type type, void
> *kbuffer,
>   * @kbuffer: from kmalloc or vmalloc
>   * @size: page-size multiple
>   * @send_offset: the offset (in bytes) where the send ring buffer starts,
> - * 		 should be 0 for BUFFER type gpadl
> + *              should be 0 for BUFFER type gpadl
>   * @gpadl_handle: some funky thing
>   */
>  static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index 11170d9a2e1a..3760cbb6ffaf 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -28,7 +28,7 @@ struct vmbus_connection vmbus_connection = {
>  	.conn_state		= DISCONNECTED,
>  	.next_gpadl_handle	= ATOMIC_INIT(0xE1E10),
> 
> -	.ready_for_suspend_event= COMPLETION_INITIALIZER(
> +	.ready_for_suspend_event = COMPLETION_INITIALIZER(
>  				  vmbus_connection.ready_for_suspend_event),
>  	.ready_for_resume_event	= COMPLETION_INITIALIZER(
>  				  vmbus_connection.ready_for_resume_event),
> --
> 2.25.1