[U-Boot] [PATCH 0/3] 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(-)
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);
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);
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; }
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
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);
memset(common, 0, sizeof common);
memset(common, 0, sizeof(*common));
Acked-by: Lukasz Majewski l.majewski@samsung.com
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)
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
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!
participants (4)
-
Jeroen Hofstee -
Lukasz Majewski -
Marek Vasut -
Tom Rini