[PATCH 2/8] staging: hv: Changed |typedef RING_BUFFER| to |struct hv_ring_buffer| throughout.

[PATCH 2/8] staging: hv: Changed |typedef RING_BUFFER| to |struct hv_ring_buffer| throughout.

[Steve Friedl]

The style guide (and Greg's prior work) have suggested that the typedef
UPPERCASENAME style is not appropriate in the Linux kernel (even though
it's been de rigueur in Win32 development for 20 years), so I'm fixing
a couple more of them that remain... one at a time.

~~~ Steve

Signed-off-by: Steve Friedl <steve@unixwiz.net>

---
 drivers/staging/hv/RingBuffer.c |    6 +++---
 drivers/staging/hv/RingBuffer.h |    6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/hv/RingBuffer.c b/drivers/staging/hv/RingBuffer.c
index f69ae33..ffd64ac 100644
--- a/drivers/staging/hv/RingBuffer.c
+++ b/drivers/staging/hv/RingBuffer.c
@@ -295,15 +295,15 @@ Description:
 --*/
 int RingBufferInit(RING_BUFFER_INFO *RingInfo, void *Buffer, u32 BufferLen)
 {
-	ASSERT(sizeof(RING_BUFFER) == PAGE_SIZE);
+	ASSERT(sizeof(struct hv_ring_buffer) == PAGE_SIZE);
 
 	memset(RingInfo, 0, sizeof(RING_BUFFER_INFO));
 
-	RingInfo->RingBuffer = (RING_BUFFER*)Buffer;
+	RingInfo->RingBuffer = (struct hv_ring_buffer*)Buffer;
 	RingInfo->RingBuffer->ReadIndex = RingInfo->RingBuffer->WriteIndex = 0;
 
 	RingInfo->RingSize = BufferLen;
-	RingInfo->RingDataSize = BufferLen - sizeof(RING_BUFFER);
+	RingInfo->RingDataSize = BufferLen - sizeof(struct hv_ring_buffer);
 
 	spin_lock_init(&RingInfo->ring_lock);
 
diff --git a/drivers/staging/hv/RingBuffer.h b/drivers/staging/hv/RingBuffer.h
index 6202157..1ea2ced 100644
--- a/drivers/staging/hv/RingBuffer.h
+++ b/drivers/staging/hv/RingBuffer.h
@@ -27,7 +27,7 @@
 
 #include <linux/scatterlist.h>
 
-typedef struct _RING_BUFFER {
+struct hv_ring_buffer {
 	/* Offset in bytes from the start of ring data below */
 	volatile u32 WriteIndex;
 
@@ -51,10 +51,10 @@ typedef struct _RING_BUFFER {
 	 * !!! DO NOT place any fields below this !!!
 	 */
 	u8 Buffer[0];
-} __attribute__((packed)) RING_BUFFER;
+} __attribute__((packed));
 
 typedef struct _RING_BUFFER_INFO {
-	RING_BUFFER *RingBuffer;
+	struct hv_ring_buffer *RingBuffer;
 	u32 RingSize;			/* Include the shared header */
 	spinlock_t ring_lock;
 
-- 
1.6.5.rc1

Re: [PATCH 2/8] staging: hv: Changed |typedef RING_BUFFER| to |struct hv_ring_buffer| throughout.

[Arnd Bergmann]

On Tuesday 22 September 2009, Steve Friedl wrote:
> The style guide (and Greg's prior work) have suggested that the typedef
> UPPERCASENAME style is not appropriate in the Linux kernel (even though
> it's been de rigueur in Win32 development for 20 years), so I'm fixing
> a couple more of them that remain... one at a time.
> 
> ~~~ Steve
> 
> Signed-off-by: Steve Friedl <steve@unixwiz.net>

A few comments about your submission, not so much about this changes
in this patch specifically.

First of all, welcome here and thanks for your first submission!
Content-wise, all your patches look correct so far, nice work.

The patch comment above mixes both information about the contents of the
patch and information about the submission. The latter (as well as your
email signature) have no place in the changeset comment above the
'---' line but should go below it. They are good to have in the mail
but serve no purpose in the git history afterwards. As an example, you
could have written the same introduction as:

|[PATCH 2/8] staging: hv: Change typedef RING_BUFFER to struct hv_ring_buffer
|
|The typedef UPPERCASENAME style is not appropriate in the Linux kernel
|(even though it's been de rigueur in Win32 development for 20 years),
|so change 'typedef RING_BUFFER' to 'struct hv_ring_buffer'.
|
|Signed-off-by: Steve Friedl <steve@unixwiz.net>
|
|---
| The style guide (and Greg's prior work) have suggested doing that,
| and I'm fixing a couple more of them that remain... one at a time.
|
| ~~~ Steve
|
| drivers/staging/hv/RingBuffer.c |    6 +++---
| drivers/staging/hv/RingBuffer.h |    6 +++---
| 2 files changed, 6 insertions(+), 6 deletions(-)

It's also preferred to have the full series in a single email thread,
with all patches as a reply to the introductory mail. git-send-email
does that with the '--thread --no-chain-reply-to', git-format-patch
does the same with 'thread=shallow' (great job on consistency ;-) ).

You should also Cc the right mailing lists and people for the subsystem
you are patching. If you don't know them, use scripts/get_maintainer.pl.
'scripts/get_maintainer.pl -f drivers/staging/hv/*' currently returns
Greg Kroah-Hartman <gregkh@suse.de>
Nicolas Palix <npalix@diku.dk>
Hank Janssen <hjanssen@microsoft.com>
Haiyang Zhang <haiyangz@microsoft.com>
Bill Pemberton <wfp5p@virginia.edu>
Randy Dunlap <randy.dunlap@oracle.com>
Jan Beulich <jbeulich@novell.com>
Moritz Muehlenhoff <jmm@debian.org>
devel@driverdev.osuosl.org
linux-kernel@vger.kernel.org

By looking at 'git log drivers/staging/hv', you can also determine that
Randy, Jan and Moritz each only contributed a single patch, so they
are not likely to be interested in your changes, but it would not
be harmful to Cc them.

Also, you should check if there is a maintainer tree that already addresses
the problems you are fixing or if there are possibly conflicts with other
changes in that tree, not sure if you did that but I've seen a number of
similar patches for the hv drivers so it would be possible that someone
else did the same already.

	Arnd <><

Re: [PATCH 2/8] staging: hv: Changed |typedef RING_BUFFER| to |struct hv_ring_buffer| throughout.

[Greg KH]

On Mon, Sep 21, 2009 at 10:40:26PM -0700, Steve Friedl wrote:
> The style guide (and Greg's prior work) have suggested that the typedef
> UPPERCASENAME style is not appropriate in the Linux kernel (even though
> it's been de rigueur in Win32 development for 20 years), so I'm fixing
> a couple more of them that remain... one at a time.
> 
> ~~~ Steve
> 
> Signed-off-by: Steve Friedl <steve@unixwiz.net>
> 
> ---
>  drivers/staging/hv/RingBuffer.c |    6 +++---

I'd rather not polish this turd anymore.  Can someone just replace the
ringbuffer.c code by using the in-kernel ringbuffer or kfifo code
instead?  I'd love to just delete this file completly, it does not
belong in the tree at all.

thanks,

greg k-h