2005-01-07 23:04:53

by Mike Miller

[permalink] [raw]
Subject: [PATCH 2.6] cciss typo fix

This patch fixes a stupid typo. I thought I submitted this but I don't seem to
find it anywhere.

cciss.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

Signed-off-by: Mike Miller <[email protected]>
-------------------------------------------------------------------------------
diff -burNp lx2610-p001/drivers/block/cciss.c lx2610/drivers/block/cciss.c
--- lx2610-p001/drivers/block/cciss.c 2005-01-07 15:29:29.645792288 -0600
+++ lx2610/drivers/block/cciss.c 2005-01-07 15:29:59.490255240 -0600
@@ -1505,8 +1505,8 @@ cciss_read_capacity(int ctlr, int logvol
return_code = sendcmd(CCISS_READ_CAPACITY,
ctlr, buf, sizeof(*buf), 1, logvol, 0, NULL, TYPE_CMD);
if (return_code == IO_OK) {
- *total_size = be32_to_cpu(*((__be32 *) &buf->total_size[0]))+1;
- *block_size = be32_to_cpu(*((__be32 *) &buf->block_size[0]));
+ *total_size = be32_to_cpu(*((__u32 *) &buf->total_size[0]))+1;
+ *block_size = be32_to_cpu(*((__u32 *) &buf->block_size[0]));
} else { /* read capacity command failed */
printk(KERN_WARNING "cciss: read capacity failed\n");
*total_size = 0;


2005-01-07 23:28:00

by Mike Miller

[permalink] [raw]
Subject: RE: [PATCH 2.6] cciss typo fix

> -----Original Message-----
> From: James Bottomley [mailto:[email protected]]
>
>
> On Fri, 2005-01-07 at 17:01 -0600, [email protected] wrote:
> > - *total_size = be32_to_cpu(*((__be32 *)
> &buf->total_size[0]))+1;
> > - *block_size = be32_to_cpu(*((__be32 *)
> &buf->block_size[0]));
> > + *total_size = be32_to_cpu(*((__u32 *)
> &buf->total_size[0]))+1;
> > + *block_size = be32_to_cpu(*((__u32 *)
> &buf->block_size[0]));
>
> I don't think that's a typo. It was introduced by this patch:
>
> ChangeSet 1.1988.24.79 2004/10/06 07:55:02 viro@http://www.linux.org.uk
> [PATCH] cciss endianness and iomem annotations
>
> The idea being that BE and LE numbers should be annotated differently,
> so the __be32 annotations look correct to me. I think sparse
> will warn
> if you make this change.

Hmmm, SuSE complained that __be32 was not defined in the kernel. Any other thoughts, anyone?

mikem

2005-01-07 23:37:54

by Mike Miller

[permalink] [raw]
Subject: RE: [PATCH 2.6] cciss typo fix

> -----Original Message-----
> From: James Bottomley [mailto:[email protected]]
> Erm, well it might not be defined in the SuSE kernel ...
> that's based on
> 2.6.5, I believe. These symbols are definitely defined in
> the mainline
> kernel.

OK, then it looks like this isn't needed.

2005-01-07 23:43:13

by James Bottomley

[permalink] [raw]
Subject: RE: [PATCH 2.6] cciss typo fix

On Fri, 2005-01-07 at 17:24 -0600, Miller, Mike (OS Dev) wrote:
> Hmmm, SuSE complained that __be32 was not defined in the kernel. Any other thoughts, anyone?

Erm, well it might not be defined in the SuSE kernel ... that's based on
2.6.5, I believe. These symbols are definitely defined in the mainline
kernel.

James


2005-01-08 00:22:56

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH 2.6] cciss typo fix

On Fri, 2005-01-07 at 17:01 -0600, [email protected] wrote:
> - *total_size = be32_to_cpu(*((__be32 *) &buf->total_size[0]))+1;
> - *block_size = be32_to_cpu(*((__be32 *) &buf->block_size[0]));
> + *total_size = be32_to_cpu(*((__u32 *) &buf->total_size[0]))+1;
> + *block_size = be32_to_cpu(*((__u32 *) &buf->block_size[0]));

I don't think that's a typo. It was introduced by this patch:

ChangeSet 1.1988.24.79 2004/10/06 07:55:02 viro@http://www.linux.org.uk
[PATCH] cciss endianness and iomem annotations

The idea being that BE and LE numbers should be annotated differently,
so the __be32 annotations look correct to me. I think sparse will warn
if you make this change.

James


2005-01-08 09:39:30

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 2.6] cciss typo fix

On Fri, Jan 07 2005, Miller, Mike (OS Dev) wrote:
> > -----Original Message-----
> > From: James Bottomley [mailto:[email protected]]
> >
> >
> > On Fri, 2005-01-07 at 17:01 -0600, [email protected] wrote:
> > > - *total_size = be32_to_cpu(*((__be32 *)
> > &buf->total_size[0]))+1;
> > > - *block_size = be32_to_cpu(*((__be32 *)
> > &buf->block_size[0]));
> > > + *total_size = be32_to_cpu(*((__u32 *)
> > &buf->total_size[0]))+1;
> > > + *block_size = be32_to_cpu(*((__u32 *)
> > &buf->block_size[0]));
> >
> > I don't think that's a typo. It was introduced by this patch:
> >
> > ChangeSet 1.1988.24.79 2004/10/06 07:55:02 viro@http://www.linux.org.uk
> > [PATCH] cciss endianness and iomem annotations
> >
> > The idea being that BE and LE numbers should be annotated differently,
> > so the __be32 annotations look correct to me. I think sparse
> > will warn
> > if you make this change.
>
> Hmmm, SuSE complained that __be32 was not defined in the kernel. Any
> other thoughts, anyone?

Hmm odd, no one should have complained, it should just have been added
to the compat header.

--
Jens Axboe

2005-01-10 15:47:16

by Mike Miller

[permalink] [raw]
Subject: RE: [PATCH 2.6] cciss typo fix

> > > - *block_size = be32_to_cpu(*((__be32 *)
> > &buf->block_size[0]));
> > > + *total_size = be32_to_cpu(*((__u32 *)
> > &buf->total_size[0]))+1;
>


> From: Jens Axboe [mailto:[email protected]]
> Hmm odd, no one should have complained, it should just have been added
> to the compat header.

Even if it were added to the compat header; is using __be32 correct in this context?

mikem

2005-01-10 15:53:21

by James Bottomley

[permalink] [raw]
Subject: RE: [PATCH 2.6] cciss typo fix

On Mon, 2005-01-10 at 09:45 -0600, Miller, Mike (OS Dev) wrote:
> Even if it were added to the compat header; is using __be32 correct in this context?

They should be. The __be annotations track unconverted big endian
numbers. be32_to_cpu checks that it's only taking __be annotated
variables (at least when sparse tracks it), so the cast is correct and
prevents sparse from warning.

James