Uninitialized scalar variable with CID 147694-93-92 #5252
Conversation
|
@heary-cao, thanks for your PR! By analyzing the history of the files in this pull request, we identified @behlendorf, @legend-hua and @ironMann to be potential reviewers. |
heary-cao
force-pushed
the
Uninitialized
branch
from
c63a27e to
174a1bd
Compare
| @@ -64,7 +64,7 @@ main(int argc, char **argv) | |||
| offset_t llseek_ret = 0; | |||
| int write_ret = 0; | |||
| int err = 0; | |||
| char mybuf[5]; | |||
| char mybuf[5] = {0}; | |||
There was a problem hiding this comment.
Since this byte gets appended to a file I'd suggest initializing it with a pattern like "abcd\0". This will make it easier to spot check with a text editor if we ever need too. It's also a little weird this array is bigger than it needs to be but let's just leave that as is. It's no doing any harm.
| @@ -67,7 +67,7 @@ int | |||
| main(int argc, char **argv) | |||
| { | |||
| int fd; | |||
| char buf[BUFSIZ]; | |||
| char buf[BUFSIZ] = {0}; | |||
There was a problem hiding this comment.
Using BUFSIZ here probably isn't a great idea, even if that is what the existing code does. How about we just make this 1024 and initialize it with memset() with a fixed character pattern. This should be fine since the intent in to overlap the IO with the 8192 mmaps region.
char buf[1024];
memset(buf, 'a', sizeof (buf));
There was a problem hiding this comment.
@behlendorf thank you for suggest.
please review again.
| @@ -89,10 +90,12 @@ main(int argc, char *argv[]) | |||
| } | |||
|
|
|||
| buf = (char *)malloc(filesize); | |||
| memset(buf, 0x0, filesize); | |||
There was a problem hiding this comment.
Normally I'd suggest using calloc() here since you'll get an optimized version which is already zero filled. But instead I think writing a known pattern in better in this case. How about memset(buf, 'a', filesize).
There was a problem hiding this comment.
@behlendorf thank you for suggest.
please review again.
heary-cao
force-pushed
the
Uninitialized
branch
2 times, most recently
from
7d3cbe2 to
05432de
Compare
| err = errno; | ||
| goto out; | ||
| free(testfile); | ||
| return errno; |
There was a problem hiding this comment.
This isn't quite right. errno may be changed by the call the free(). In fact, errno may be changed by perror and it should be saved before it's called, see perror(3). Also there's a cstyle issue regarding return, it should be return (errno). This first block should be changed to:
err = errno;
perror(...);
free(testfile);
return (err);
And the subsequent goto blocks should be:
err = errno;
perror(...);
goto out;
| @@ -88,11 +89,13 @@ main(int argc, char *argv[]) | |||
| return (1); | |||
| } | |||
|
|
|||
| buf = (char *)malloc(filesize); | |||
| buf = (char *)calloc(1, filesize); | |||
There was a problem hiding this comment.
There needs to be a NULL check here since calloc may fail.
issues: fix coverity defects coverity scan CID:147694, Type:Uninitialized scalar variable coverity scan CID:147693, Type:Uninitialized scalar variable coverity scan CID:147692, Type:Uninitialized scalar variable Signed-off-by: cao.xuewen <[email protected]>
heary-cao
force-pushed
the
Uninitialized
branch
from
05432de to
738943f
Compare
issues:
fix coverity defects
coverity scan CID:147694, Type:Uninitialized scalar variable
coverity scan CID:147693, Type:Uninitialized scalar variable
coverity scan CID:147692, Type:Uninitialized scalar variable
Signed-off-by: cao.xuewen [email protected]