[U-Boot] [PATCH 0/3] fix clang warnings about sizeof usage

[U-Boot] [PATCH 0/3] fix clang warnings about sizeof usage

[Jeroen Hofstee]

The following patches prevent some warnings with clang in
case a struct/buffer is not properly cleared by incorrect
sizeof usage. For completeness, I have not tested it on hw!

Jeroen Hofstee (3):
  usb:g_dnl:f_thor: remove memset before memcpy
  usb:composite: clear the whole common buffer
  ext4: correctly zero filename

 drivers/usb/gadget/f_mass_storage.c | 4 ++--
 drivers/usb/gadget/f_thor.c         | 1 -
 fs/ext4/ext4_write.c                | 2 +-
 3 files changed, 3 insertions(+), 4 deletions(-)

-- 
1.8.3.2

[U-Boot] [PATCH 1/3] usb:g_dnl:f_thor: remove memset before memcpy

[Jeroen Hofstee]

since ALLOC_CACHE_ALIGN_BUFFER defines a pointer and not a
buffer, the memset with sizeof(rqt) likely does something else
then intended. Since there is a memcpy directly after it with
the full size, drop the memset completely.

Cc: Lukasz Majewski <l.majewski@samsung.com>
Cc: Marek Vasut <marex@denx.de>
Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl>
---
 drivers/usb/gadget/f_thor.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/gadget/f_thor.c b/drivers/usb/gadget/f_thor.c
index 28f215e..4e06273 100644
--- a/drivers/usb/gadget/f_thor.c
+++ b/drivers/usb/gadget/f_thor.c
@@ -306,7 +306,6 @@ static int process_data(void)
 	ALLOC_CACHE_ALIGN_BUFFER(struct rqt_box, rqt, sizeof(struct rqt_box));
 	int ret = -EINVAL;
 
-	memset(rqt, 0, sizeof(rqt));
 	memcpy(rqt, thor_rx_data_buf, sizeof(struct rqt_box));
 
 	debug("+RQT: %d, %d\n", rqt->rqt, rqt->rqt_data);
-- 
1.8.3.2

[U-Boot] [PATCH 2/3] usb:composite: clear the whole common buffer

[Jeroen Hofstee]

Since the struct fsg_common is calloced, reset it completely
with zero's when reused. While at it, make checkpatch happy.

cc: Lukasz Majewski <l.majewski@samsung.com>
cc: Piotr Wilczek <p.wilczek@samsung.com>
cc: Kyungmin Park <kyungmin.park@samsung.com>
cc: Marek Vasut <marek.vasut@gmail.com>
Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl>
---
drivers/usb/gadget/f_mass_storage.c:2470:28: warning: 'memset' call
operates on objects of type 'struct fsg_common' while the size is
based on a different type 'struct fsg_common *' [-Wsizeof-pointer-memaccess]
                memset(common, 0, sizeof common);

Note: There is another warning worth mentioning, but I don't
know what the correct behaviour should be.

drivers/usb/gadget/f_mass_storage.c:1153:6: warning: variable
'sdinfo' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
        if (!curlun) {          /* Unsupported LUNs are okay */
            ^~~~~~~
drivers/usb/gadget/f_mass_storage.c:1168:21: note: uninitialized use occurs here
        put_unaligned_be32(sdinfo, &buf[3]);    /* Sense information */
---
 drivers/usb/gadget/f_mass_storage.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
index 6374bb9..f274d96 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -2462,12 +2462,12 @@ static struct fsg_common *fsg_common_init(struct fsg_common *common,
 
 	/* Allocate? */
 	if (!common) {
-		common = calloc(sizeof *common, 1);
+		common = calloc(sizeof(*common), 1);
 		if (!common)
 			return ERR_PTR(-ENOMEM);
 		common->free_storage_on_release = 1;
 	} else {
-		memset(common, 0, sizeof common);
+		memset(common, 0, sizeof(*common));
 		common->free_storage_on_release = 0;
 	}
 
-- 
1.8.3.2

[U-Boot] [PATCH 3/3] ext4: correctly zero filename

[Jeroen Hofstee]

Since ALLOC_CACHE_ALIGN_BUFFER declares a char* for filename
sizeof(filename) is not the size of the buffer. Use the already
known length instead.

cc: Uma Shankar <uma.shankar@samsung.com>
cc: Manjunatha C Achar <a.manjunatha@samsung.com>
cc: Marek Vasut <marek.vasut@gmail.com>
Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl>
---
 fs/ext4/ext4_write.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
index c42add9..648a596 100644
--- a/fs/ext4/ext4_write.c
+++ b/fs/ext4/ext4_write.c
@@ -840,7 +840,7 @@ int ext4fs_write(const char *fname, unsigned char *buffer,
 	unsigned int ibmap_idx;
 	struct ext_filesystem *fs = get_fs();
 	ALLOC_CACHE_ALIGN_BUFFER(char, filename, 256);
-	memset(filename, 0x00, sizeof(filename));
+	memset(filename, 0x00, 256);
 
 	g_parent_inode = zalloc(sizeof(struct ext2_inode));
 	if (!g_parent_inode)
-- 
1.8.3.2

[U-Boot] [PATCH 3/3] ext4: correctly zero filename

[Marek Vasut]

On Monday, June 09, 2014 at 03:29:00 PM, Jeroen Hofstee wrote:
> Since ALLOC_CACHE_ALIGN_BUFFER declares a char* for filename
> sizeof(filename) is not the size of the buffer. Use the already
> known length instead.
> 
> cc: Uma Shankar <uma.shankar@samsung.com>
> cc: Manjunatha C Achar <a.manjunatha@samsung.com>
> cc: Marek Vasut <marek.vasut@gmail.com>
> Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl>

Nice :-)

Acked-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

[U-Boot] [PATCH 2/3] usb:composite: clear the whole common buffer

[Marek Vasut]

On Monday, June 09, 2014 at 03:28:59 PM, Jeroen Hofstee wrote:
> Since the struct fsg_common is calloced, reset it completely
> with zero's when reused. While at it, make checkpatch happy.
> 
> cc: Lukasz Majewski <l.majewski@samsung.com>
> cc: Piotr Wilczek <p.wilczek@samsung.com>
> cc: Kyungmin Park <kyungmin.park@samsung.com>
> cc: Marek Vasut <marek.vasut@gmail.com>
> Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl>

Acked-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

[U-Boot] [PATCH 2/3] usb:composite: clear the whole common buffer

[Lukasz Majewski]

Hi Jeroen,

> Since the struct fsg_common is calloced, reset it completely
> with zero's when reused. While at it, make checkpatch happy.
> 
> cc: Lukasz Majewski <l.majewski@samsung.com>
> cc: Piotr Wilczek <p.wilczek@samsung.com>
> cc: Kyungmin Park <kyungmin.park@samsung.com>
> cc: Marek Vasut <marek.vasut@gmail.com>
> Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl>
> ---
> drivers/usb/gadget/f_mass_storage.c:2470:28: warning: 'memset' call
> operates on objects of type 'struct fsg_common' while the size is
> based on a different type 'struct fsg_common
> *' [-Wsizeof-pointer-memaccess] memset(common, 0, sizeof common);
> 
> Note: There is another warning worth mentioning, but I don't
> know what the correct behaviour should be.
> 
> drivers/usb/gadget/f_mass_storage.c:1153:6: warning: variable
> 'sdinfo' is used uninitialized whenever 'if' condition is false
> [-Wsometimes-uninitialized] if (!curlun) {          /* Unsupported
> LUNs are okay */ ^~~~~~~
> drivers/usb/gadget/f_mass_storage.c:1168:21: note: uninitialized use
> occurs here put_unaligned_be32(sdinfo, &buf[3]);    /* Sense
> information */ ---
>  drivers/usb/gadget/f_mass_storage.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/f_mass_storage.c
> b/drivers/usb/gadget/f_mass_storage.c index 6374bb9..f274d96 100644
> --- a/drivers/usb/gadget/f_mass_storage.c
> +++ b/drivers/usb/gadget/f_mass_storage.c
> @@ -2462,12 +2462,12 @@ static struct fsg_common
> *fsg_common_init(struct fsg_common *common, 
>  	/* Allocate? */
>  	if (!common) {
> -		common = calloc(sizeof *common, 1);
> +		common = calloc(sizeof(*common), 1);
>  		if (!common)
>  			return ERR_PTR(-ENOMEM);
>  		common->free_storage_on_release = 1;
>  	} else {
> -		memset(common, 0, sizeof common);
> +		memset(common, 0, sizeof(*common));
>  		common->free_storage_on_release = 0;
>  	}
>  

Acked-by: Lukasz Majewski <l.majewski@samsung.com>

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

[U-Boot] [PATCH 1/3] usb:g_dnl:f_thor: remove memset before memcpy

[Lukasz Majewski]

Hi Jeroen,

> since ALLOC_CACHE_ALIGN_BUFFER defines a pointer and not a
> buffer, the memset with sizeof(rqt) likely does something else
> then intended. Since there is a memcpy directly after it with
> the full size, drop the memset completely.

Acked-by: Lukasz Majewski <l.majewski@samsung.com>

> 
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> Cc: Marek Vasut <marex@denx.de>
> Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl>
> ---
>  drivers/usb/gadget/f_thor.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/f_thor.c b/drivers/usb/gadget/f_thor.c
> index 28f215e..4e06273 100644
> --- a/drivers/usb/gadget/f_thor.c
> +++ b/drivers/usb/gadget/f_thor.c
> @@ -306,7 +306,6 @@ static int process_data(void)
>  	ALLOC_CACHE_ALIGN_BUFFER(struct rqt_box, rqt, sizeof(struct
> rqt_box)); int ret = -EINVAL;
>  
> -	memset(rqt, 0, sizeof(rqt));
>  	memcpy(rqt, thor_rx_data_buf, sizeof(struct rqt_box));
>  
>  	debug("+RQT: %d, %d\n", rqt->rqt, rqt->rqt_data);


-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

[U-Boot] [U-Boot,3/3] ext4: correctly zero filename

[Tom Rini]

On Mon, Jun 09, 2014 at 03:29:00PM +0200, Jeroen Hofstee wrote:

> Since ALLOC_CACHE_ALIGN_BUFFER declares a char* for filename
> sizeof(filename) is not the size of the buffer. Use the already
> known length instead.
> 
> cc: Uma Shankar <uma.shankar@samsung.com>
> cc: Manjunatha C Achar <a.manjunatha@samsung.com>
> cc: Marek Vasut <marek.vasut@gmail.com>
> Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl>
> Acked-by: Marek Vasut <marex@denx.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140611/a33b8196/attachment.pgp>