fix clang warnings about sizeof usage

PATCH New Jeroen Hofstee 4 participants 9 messages Jun 9, 2014 – Jun 11, 2014

Subsystems (2)

USB ext4 file system

lore

Share & tools

lore

Jeroen Hofstee

author

2014-06-09 13:28 UTC #0 lore raw

cover fix clang warnings about sizeof usage

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(-)

Signature · 1 line

1.8.3.2

Jeroen Hofstee

author

2014-06-09 13:28 UTC #1 lore raw

PATCH 1/3 usb:g_dnl:f_thor: remove memset before memcpy

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 <[email protected]>

Cc Marek Vasut <[email protected]>

Signed-off-by Jeroen Hofstee <[email protected]>

---
 drivers/usb/gadget/f_thor.c | 1 -
 1 file changed, 1 deletion(-)

drivers/usb/gadget/f_thor.c +0 -1

index 28f215e..4e06273 100644
@@ -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);

Signature · 1 line

1.8.3.2

Lukasz Majewski

2014-06-10 06:50 UTC #2 lore raw

PATCH 1/3 usb:g_dnl:f_thor: remove memset before memcpy

Hi Jeroen,

Quoted 4 lines

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 <[email protected]>

Quoted 20 lines

Cc Lukasz Majewski <[email protected]>

Cc Marek Vasut <[email protected]>

Signed-off-by Jeroen Hofstee <[email protected]>

---
 drivers/usb/gadget/f_thor.c | 1 -
 1 file changed, 1 deletion(-)

drivers/usb/gadget/f_thor.c +0 -1

index 28f215e..4e06273 100644
@@ -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);

Signature · 5 lines

Best regards,

Lukasz Majewski

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

Jeroen Hofstee

author

2014-06-09 13:28 UTC #3 lore raw

PATCH 2/3 usb:composite: clear the whole common buffer

In reply to 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 <[email protected]>

cc Piotr Wilczek <[email protected]>

cc Kyungmin Park <[email protected]>

cc Marek Vasut <[email protected]>

Signed-off-by Jeroen Hofstee <[email protected]>

---
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(-)

drivers/usb/gadget/f_mass_storage.c +2 -2

index 6374bb9..f274d96 100644
@@ -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;
 	}

Signature · 1 line

1.8.3.2

Marek Vasut

2014-06-09 17:59 UTC #4 lore raw

PATCH 2/3 usb:composite: clear the whole common buffer

Quoted 8 lines · 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 <[email protected]>
cc Piotr Wilczek <[email protected]>
cc Kyungmin Park <[email protected]>
cc Marek Vasut <[email protected]>
Signed-off-by Jeroen Hofstee <[email protected]>

Acked-by Marek Vasut <[email protected]>

Best regards,
Marek Vasut

Lukasz Majewski

2014-06-10 06:47 UTC #5 lore raw

PATCH 2/3 usb:composite: clear the whole common buffer

In reply to Jeroen Hofstee

Hi Jeroen,

Quoted 36 lines

Since the struct fsg_common is calloced, reset it completely with zero's when reused. While at it, make checkpatch happy.
cc Lukasz Majewski <[email protected]>
cc Piotr Wilczek <[email protected]>
cc Kyungmin Park <[email protected]>
cc Marek Vasut <[email protected]>
Signed-off-by Jeroen Hofstee <[email protected]>
---
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(-)
(quoted diff) +0 -0
b/drivers/usb/gadget/f_mass_storage.c index 6374bb9..f274d96 100644
drivers/usb/gadget/f_mass_storage.c +0 -1
@@ -2462,12 +2462,12 @@ static struct fsg_common
 *fsg_common_init(struct fsg_common *common, 
 	/* Allocate? */
 	if (!common) {
-		common = calloc(sizeof *common, 1);

drivers/usb/gadget/f_mass_storage.c static struct fsg_common · line 2473

Quoted 10 lines

drivers/usb/gadget/f_mass_storage.c +2 -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 <[email protected]>

Signature · 5 lines

Best regards,

Lukasz Majewski

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

Jeroen Hofstee

author

2014-06-09 13:29 UTC #6 lore raw

PATCH 3/3 ext4: correctly zero filename

In reply to 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 <[email protected]>

cc Manjunatha C Achar <[email protected]>

cc Marek Vasut <[email protected]>

Signed-off-by Jeroen Hofstee <[email protected]>

---
 fs/ext4/ext4_write.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

fs/ext4/ext4_write.c +1 -1

index c42add9..648a596 100644
@@ -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)

Signature · 1 line

1.8.3.2

Marek Vasut

2014-06-09 17:58 UTC #7 lore raw

PATCH 3/3 ext4: correctly zero filename

Quoted 8 lines · 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 <[email protected]>
cc Manjunatha C Achar <[email protected]>
cc Marek Vasut <[email protected]>
Signed-off-by Jeroen Hofstee <[email protected]>

Nice :-)

Acked-by Marek Vasut <[email protected]>

Best regards,
Marek Vasut

Tom Rini

2014-06-11 22:19 UTC #8 lore raw

PATCH 3/3 ext4: correctly zero filename

In reply to Jeroen Hofstee

Quoted 9 lines · 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 <[email protected]>
cc Manjunatha C Achar <[email protected]>
cc Marek Vasut <[email protected]>
Signed-off-by Jeroen Hofstee <[email protected]>
Acked-by Marek Vasut <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines

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>