On Tue, May 05, 2015 at 06:36:17PM -0400, Benjamin Romer wrote:
> From: Prarit Bhargava <prarit@xxxxxxxxxx>
> diff --git a/drivers/staging/unisys/visorbus/visorchannel.c b/drivers/staging/unisys/visorbus/visorchannel.c
> index 33a4360..ff14a0d 100644
> --- a/drivers/staging/unisys/visorbus/visorchannel.c
> +++ b/drivers/staging/unisys/visorbus/visorchannel.c
> @@ -55,60 +55,52 @@ visorchannel_create_guts(HOSTADDRESS physaddr, ulong channel_bytes,
> struct visorchannel *parent, ulong off, uuid_le guid,
> BOOL needs_lock)
> - void *rc = NULL;
> + struct visorchannel *channel;
> + int err;
> + size_t size = sizeof(struct channel_header);
This patch is fine and I was going to ignore some minor nits but then
they became a habbit in later patches. Don't make "size" a variable
for no reason (I guess the reason is that the original code didn't fit
into the 80 character limit but that doesn't count as a valid reason).
It just means we have to scroll up to find what "size" is. "size" is a
generic meaningless name. This patch doesn't even use it consistently.
> + err = visor_memregion_read(channel->memregion, 0, &channel->chan_hdr,
> + sizeof(struct channel_header));
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
See? This one wasn't updated.
Also there were some unrelated changes. Benjamin, you should have
complained about those when the patch was first sent. That's like the
most obvious thing to check when you're reviewing patches. Now it's an
awkward thing because there are 101 more patches on top of it.
Anyway, let's merge this because it's a very minor thing.
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
