Question about switch() of soc_mbus_config_compatible()

Question about switch() of soc_mbus_config_compatible()

[Kuninori Morimoto]

Hi Mauro, Guennadi

I would like to ask you about switch() of
linux/drivers/media/platform/soc_camera/soc_mediabus.c :: soc_mbus_config_compatible

unsigned int soc_mbus_config_compatible(const struct v4l2_mbus_config *cfg,
					unsigned int flags)
{
	...

	switch (cfg->type) {
	case V4L2_MBUS_PARALLEL:
		hsync = common_flags & (V4L2_MBUS_HSYNC_ACTIVE_HIGH |
					V4L2_MBUS_HSYNC_ACTIVE_LOW);
		vsync = common_flags & (V4L2_MBUS_VSYNC_ACTIVE_HIGH |
=>					V4L2_MBUS_VSYNC_ACTIVE_LOW);
	case V4L2_MBUS_BT656:
		pclk = common_flags & (V4L2_MBUS_PCLK_SAMPLE_RISING |
				       V4L2_MBUS_PCLK_SAMPLE_FALLING);
		data = common_flags & (V4L2_MBUS_DATA_ACTIVE_HIGH |
				       V4L2_MBUS_DATA_ACTIVE_LOW);
		mode = common_flags & (V4L2_MBUS_MASTER | V4L2_MBUS_SLAVE);
		return (!hsync || !vsync || !pclk || !data || !mode) ?
			0 : common_flags;
	...
}

Here, there is no break, no return, no /* FALL THROUGH */
It is very confusable, but what is this intention ?

[PATCH 0/2][RFC] mmc: cast to avoid unexpected error

[Kuninori Morimoto]

Hi Ulf

These 2 patches adds cast to avoid unexpected error.
It tries copy to u64 without cast.
The data will be 0xfff... if 1st bit was 1.
These are reported by coverity tool, but I couldn't test it.
So, I added [RFC] on these patches.
I'm happy if someone tests it, or can get deep review.

Kuninori Morimoto (2):
      mmc: cast u8 to unsigned long long to avoid unexpected error
      mmc: cast unsigned int to typeof(sector_t) to avoid unexpected error

 drivers/mmc/card/block.c | 2 +-
 drivers/mmc/core/mmc.c   | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

[PATCH 1/2][RFC] mmc: cast u8 to unsigned long long to avoid unexpected error

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

card->ext_csd.enhanced_area_offset is defined as "unsigned long long",
and, ext_csd[] is defined as u8.
unsigned long long data might have strange data if first bit of ext_csd[]
was 1. this patch cast it to (unsigned long long)
ex)
        u8  data8;
        u64 data64;

        data8 = 0x80;
        data64 = (data8 << 24); // 0xffffffff80000000
        data64 = (((unsigned long long)data8) << 24); // 0x80000000;

Reported-by: coverity <http://www.coverity.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/mmc/core/mmc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 1d41e85..3c663a2 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -265,8 +265,10 @@ static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
 			 * calculate the enhanced data area offset, in bytes
 			 */
 			card->ext_csd.enhanced_area_offset =
-				(ext_csd[139] << 24) + (ext_csd[138] << 16) +
-				(ext_csd[137] << 8) + ext_csd[136];
+				(((unsigned long long)ext_csd[139]) << 24) +
+				(((unsigned long long)ext_csd[138]) << 16) +
+				(((unsigned long long)ext_csd[137]) << 8) +
+				(((unsigned long long)ext_csd[136]));
 			if (mmc_card_blockaddr(card))
 				card->ext_csd.enhanced_area_offset <<= 9;
 			/*
-- 
1.9.1

[PATCH 2/2][RFC] mmc: cast unsigned int to typeof(sector_t) to avoid unexpected error

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

card->csd.capacity is defined as "unsigned int",
and, sector_t is defined as "u64" or "unsigned long" (depends on CONFIG_LBDAF)
sector_t data might have strange data if first bit of unsigned int
was 1. this patch cast it to typeof(sector_t)

ex) if sector_t was u64

        unsigned int data;
        sector_t sector;

        data = 0x80000000;
        sector = (data << 8); // 0xffffff8000000000
        sector = (((typeof(sector_t))data) << 8); // 0x8000000000

Reported-by: coverity <http://www.coverity.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/mmc/card/block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index c69afb5..4d09b0c 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -2205,7 +2205,7 @@ static struct mmc_blk_data *mmc_blk_alloc(struct mmc_card *card)
 		 * The CSD capacity field is in units of read_blkbits.
 		 * set_capacity takes units of 512 bytes.
 		 */
-		size = card->csd.capacity << (card->csd.read_blkbits - 9);
+		size = (typeof(sector_t))card->csd.capacity << (card->csd.read_blkbits - 9);
 	}
 
 	return mmc_blk_alloc_req(card, &card->dev, size, false, NULL,
-- 
1.9.1

Re: [PATCH 2/2][RFC] mmc: cast unsigned int to typeof(sector_t) to avoid unexpected error

[Kuninori Morimoto]

Hi

> card->csd.capacity is defined as "unsigned int",
> and, sector_t is defined as "u64" or "unsigned long" (depends on CONFIG_LBDAF)
> sector_t data might have strange data if first bit of unsigned int
> was 1. this patch cast it to typeof(sector_t)
> 
> ex) if sector_t was u64
> 
>         unsigned int data;
>         sector_t sector;
> 
>         data = 0x80000000;
>         sector = (data << 8); // 0xffffff8000000000
>         sector = (((typeof(sector_t))data) << 8); // 0x8000000000

Sorry, I noticed this explain (= number of 0) was wrong
Maybe it should be

         data = 0x800000;
         sector = (data << 8); // 0xffffffff80000000
         sector = (((typeof(sector_t))data) << 8); // 0x80000000

or

         data = 0x80000000;
         sector = (data << 8); // 0x0
         sector = (((typeof(sector_t))data) << 8); // 0x8000000000

[PATCH 0/2] mmc: cast to avoid unexpected error

[Kuninori Morimoto]

Hi Ulf

These are non RFC version of mmc data cast patches
which were posted in
Subject: [PATCH 0/2][RFC] mmc: cast to avoid unexpected error
Date: Wed, 8 Apr 2015 07:32:35 +0000

These 2 patches adds cast to avoid unexpected error.
It tries copy to u64 without cast.
The data will be 0xfff... if last bit was 1.
These are reported by coverity tool.
I'm happy if someone tests it, or can get deep review.

Kuninori Morimoto (2):
      mmc: cast u8 to unsigned long long to avoid unexpected error
      mmc: cast unsigned int to typeof(sector_t) to avoid unexpected error

 drivers/mmc/card/block.c | 2 +-
 drivers/mmc/core/mmc.c   | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] mmc: cast u8 to unsigned long long to avoid unexpected error

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

card->ext_csd.enhanced_area_offset is defined as "unsigned long long",
and, ext_csd[] is defined as u8.
unsigned long long data might have strange data if first bit of ext_csd[]
was 1. this patch cast it to (unsigned long long)
ex)
        u8  data8;
        u64 data64;

        data8 = 0x80;
        data64 = (data8 << 24); // 0xffffffff80000000
        data64 = (((unsigned long long)data8) << 24); // 0x80000000;

Reported-by: coverity <http://www.coverity.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/mmc/core/mmc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index c84131e..c6bb577 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -266,8 +266,10 @@ static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
 			 * calculate the enhanced data area offset, in bytes
 			 */
 			card->ext_csd.enhanced_area_offset =
-				(ext_csd[139] << 24) + (ext_csd[138] << 16) +
-				(ext_csd[137] << 8) + ext_csd[136];
+				(((unsigned long long)ext_csd[139]) << 24) +
+				(((unsigned long long)ext_csd[138]) << 16) +
+				(((unsigned long long)ext_csd[137]) << 8) +
+				(((unsigned long long)ext_csd[136]));
 			if (mmc_card_blockaddr(card))
 				card->ext_csd.enhanced_area_offset <<= 9;
 			/*
-- 
1.9.1

[PATCH 2/2] mmc: cast unsigned int to typeof(sector_t) to avoid unexpected error

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

card->csd.capacity is defined as "unsigned int",
and, sector_t is defined as "u64" or "unsigned long" (depends on CONFIG_LBDAF)
sector_t data might have strange data if first bit of unsigned int
was 1. this patch cast it to typeof(sector_t)

ex) if sector_t was u64

        unsigned int data;
        sector_t sector;

        data = 0x800000;
        sector = (data << 8); // 0xffffffff80000000
        sector = (((typeof(sector_t))data) << 8); // 0x80000000

or

        data = 0x80000000;
        sector = (data << 8); // 0x0
        sector = (((typeof(sector_t))data) << 8); // 0x8000000000

Reported-by: coverity <http://www.coverity.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/mmc/card/block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index c69afb5..4d09b0c 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -2205,7 +2205,7 @@ static struct mmc_blk_data *mmc_blk_alloc(struct mmc_card *card)
 		 * The CSD capacity field is in units of read_blkbits.
 		 * set_capacity takes units of 512 bytes.
 		 */
-		size = card->csd.capacity << (card->csd.read_blkbits - 9);
+		size = (typeof(sector_t))card->csd.capacity << (card->csd.read_blkbits - 9);
 	}
 
 	return mmc_blk_alloc_req(card, &card->dev, size, false, NULL,
-- 
1.9.1

Re: [PATCH 0/2] mmc: cast to avoid unexpected error

[Ulf Hansson]

On 14 April 2015 at 06:06, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> Hi Ulf
>
> These are non RFC version of mmc data cast patches
> which were posted in
> Subject: [PATCH 0/2][RFC] mmc: cast to avoid unexpected error
> Date: Wed, 8 Apr 2015 07:32:35 +0000
>
> These 2 patches adds cast to avoid unexpected error.
> It tries copy to u64 without cast.
> The data will be 0xfff... if last bit was 1.
> These are reported by coverity tool.
> I'm happy if someone tests it, or can get deep review.
>
> Kuninori Morimoto (2):
>       mmc: cast u8 to unsigned long long to avoid unexpected error
>       mmc: cast unsigned int to typeof(sector_t) to avoid unexpected error
>
>  drivers/mmc/card/block.c | 2 +-
>  drivers/mmc/core/mmc.c   | 6 ++++--
>  2 files changed, 5 insertions(+), 3 deletions(-)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Please run another round of checkpatch for these.

Kind regards
Uffe

Re: [PATCH 0/2] mmc: cast to avoid unexpected error

[Kuninori Morimoto]

Hi Ulf

> > Kuninori Morimoto (2):
> >       mmc: cast u8 to unsigned long long to avoid unexpected error
> >       mmc: cast unsigned int to typeof(sector_t) to avoid unexpected error
(snip)
> Please run another round of checkpatch for these.

Thank you. will do in v2

[PATCH 0/2 v2] mmc: cast to avoid unexpected error

[Kuninori Morimoto]

Hi Ulf

These are v2 of mmc data cast patches which were posted in
Subject: [PATCH 0/2][RFC] mmc: cast to avoid unexpected error
Date: Wed, 8 Apr 2015 07:32:35 +0000

These 2 patches adds cast to avoid unexpected error.
It tries copy to u64 without cast.
The data will be 0xfff... if last bit was 1.
These are reported by coverity tool.
I'm happy if someone tests it, or can get deep review.

Kuninori Morimoto (2):
      mmc: cast u8 to unsigned long long to avoid unexpected error
      mmc: cast unsigned int to typeof(sector_t) to avoid unexpected error

 drivers/mmc/card/block.c | 2 +-
 drivers/mmc/core/mmc.c   | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/2 v2] mmc: cast to avoid unexpected error

[Kuninori Morimoto]

Hi Ulf

These are v2 of mmc data cast patches which were posted in
Subject: [PATCH 0/2][RFC] mmc: cast to avoid unexpected error
Date: Wed, 8 Apr 2015 07:32:35 +0000

These 2 patches adds cast to avoid unexpected error.
It tries copy to u64 without cast.
The data will be 0xfff... if last bit was 1.
These are reported by coverity tool.
I'm happy if someone tests it, or can get deep review.

Kuninori Morimoto (2):
      mmc: cast u8 to unsigned long long to avoid unexpected error
      mmc: cast unsigned int to typeof(sector_t) to avoid unexpected error

 drivers/mmc/card/block.c | 2 +-
 drivers/mmc/core/mmc.c   | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2 v2] mmc: cast u8 to unsigned long long to avoid unexpected error

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

card->ext_csd.enhanced_area_offset is defined as "unsigned long long",
and, ext_csd[] is defined as u8.
unsigned long long data might have strange data if first bit of ext_csd[]
was 1. this patch cast it to (unsigned long long)
Special thanks to coverity <http://www.coverity.com>

ex)
        u8  data8;
        u64 data64;

        data8 = 0x80;
        data64 = (data8 << 24); // 0xffffffff80000000
        data64 = (((unsigned long long)data8) << 24); // 0x80000000;

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v1 -> v2

 - Tidyjp log comment

 drivers/mmc/core/mmc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index f36c76f..2be4073 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -266,8 +266,10 @@ static void mmc_manage_enhanced_area(struct mmc_card *card, u8 *ext_csd)
 			 * calculate the enhanced data area offset, in bytes
 			 */
 			card->ext_csd.enhanced_area_offset =
-				(ext_csd[139] << 24) + (ext_csd[138] << 16) +
-				(ext_csd[137] << 8) + ext_csd[136];
+				(((unsigned long long)ext_csd[139]) << 24) +
+				(((unsigned long long)ext_csd[138]) << 16) +
+				(((unsigned long long)ext_csd[137]) << 8) +
+				(((unsigned long long)ext_csd[136]));
 			if (mmc_card_blockaddr(card))
 				card->ext_csd.enhanced_area_offset <<= 9;
 			/*
-- 
1.9.1

[PATCH 2/2 v2] mmc: cast unsigned int to typeof(sector_t) to avoid unexpected error

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

card->csd.capacity is defined as "unsigned int", and sector_t is defined as
"u64" or "unsigned long" (depends on CONFIG_LBDAF). Thus, sector_t data
might have strange data (see below). This patch cast it to typeof(sector_t)
Special thanks to coverity <http://www.coverity.com>

ex) if sector_t was u64

        unsigned int data;
        sector_t sector;

        data = 0x800000;
        sector = (data << 8); // 0xffffffff80000000
        sector = (((typeof(sector_t))data) << 8); // 0x80000000

or

        data = 0x80000000;
        sector = (data << 8); // 0x0
        sector = (((typeof(sector_t))data) << 8); // 0x8000000000

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v1 -> v2

 - tidyup log comment
 - tidyup line over 80 characters

 drivers/mmc/card/block.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 60f7141..029a872 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -2217,7 +2217,8 @@ static struct mmc_blk_data *mmc_blk_alloc(struct mmc_card *card)
 		 * The CSD capacity field is in units of read_blkbits.
 		 * set_capacity takes units of 512 bytes.
 		 */
-		size = card->csd.capacity << (card->csd.read_blkbits - 9);
+		size = (typeof(sector_t))card->csd.capacity
+			<< (card->csd.read_blkbits - 9);
 	}
 
 	return mmc_blk_alloc_req(card, &card->dev, size, false, NULL,
-- 
1.9.1

Re: [PATCH 0/2 v2] mmc: cast to avoid unexpected error

[Ulf Hansson]

On 11 May 2015 at 09:34, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> Hi Ulf
>
> These are v2 of mmc data cast patches which were posted in
> Subject: [PATCH 0/2][RFC] mmc: cast to avoid unexpected error
> Date: Wed, 8 Apr 2015 07:32:35 +0000
>
> These 2 patches adds cast to avoid unexpected error.
> It tries copy to u64 without cast.
> The data will be 0xfff... if last bit was 1.
> These are reported by coverity tool.
> I'm happy if someone tests it, or can get deep review.
>
> Kuninori Morimoto (2):
>       mmc: cast u8 to unsigned long long to avoid unexpected error
>       mmc: cast unsigned int to typeof(sector_t) to avoid unexpected error
>
>  drivers/mmc/card/block.c | 2 +-
>  drivers/mmc/core/mmc.c   | 6 ++++--
>  2 files changed, 5 insertions(+), 3 deletions(-)

Thanks, applied!

Kind regards
Uffe