* [PATCH ucspi-vsock 0/2] Fix clang-tidy warnings
@ 2021-03-21 14:46 Alyssa Ross
2021-03-21 14:51 ` [PATCH ucspi-vsock 1/2] exec: free argv if exec fails Alyssa Ross
2021-03-21 16:30 ` [PATCH ucspi-vsock 0/2] Fix clang-tidy warnings Cole Helbling
0 siblings, 2 replies; 5+ messages in thread
From: Alyssa Ross @ 2021-03-21 14:46 UTC (permalink / raw)
To: devel; +Cc: Cole Helbling
clang-tidy also produced this warning:
/home/src/ucspi-vsock/repro.c:43:19: warning: unused parameter 'sig' [clang-diagnostic-unused-parameter]
void sig_exit(int sig) { exit(EX_UNAVAILABLE); }
^
vsock.c:27:23: warning: The left operand of '!=' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
if (addr->svm_family != AF_VSOCK) {
^
vsock.c:90:6: note: Assuming the condition is false
if (getsockname(fd, (struct sockaddr *)&addr, &addrlen) == -1)
^
vsock.c:90:2: note: Taking false branch
if (getsockname(fd, (struct sockaddr *)&addr, &addrlen) == -1)
^
vsock.c:93:9: note: Calling 'fill_cid_and_port'
return fill_cid_and_port(&addr, cid, port);
^
vsock.c:27:23: note: The left operand of '!=' is a garbage value
if (addr->svm_family != AF_VSOCK) {
But I think this is just warning me that the POSIX socket API violates
the strict aliasing rule. (Which is true, but there's not a lot I can
do about it...)
It also tells me I should use memset_s instead of memset, but, well...
https://en.wikipedia.org/wiki/C11_(C_standard_revision)#Criticism
Alyssa Ross (2):
exec: free argv if exec fails
vsockserver: fix uninitialized variable
exec.c | 5 ++++-
vsockserver.c | 2 +-
2 files changed, 5 insertions(+), 2 deletions(-)
--
2.30.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH ucspi-vsock 1/2] exec: free argv if exec fails
2021-03-21 14:46 [PATCH ucspi-vsock 0/2] Fix clang-tidy warnings Alyssa Ross
@ 2021-03-21 14:51 ` Alyssa Ross
2021-03-21 14:54 ` [PATCH ucspi-vsock 2/2] vsockserver: fix uninitialized variable Alyssa Ross
2021-03-21 16:30 ` [PATCH ucspi-vsock 0/2] Fix clang-tidy warnings Cole Helbling
1 sibling, 1 reply; 5+ messages in thread
From: Alyssa Ross @ 2021-03-21 14:51 UTC (permalink / raw)
To: devel; +Cc: Cole Helbling
Identified by clang-tidy:
exec.c:21:2: warning: Potential leak of memory pointed to by 'argv' [clang-analyzer-unix.Malloc]
return execvp(file, argv);
^
exec.c:15:16: note: Memory is allocated
char **argv = calloc(argz_count(argz, len) + 1, sizeof(char *));
^
exec.c:16:6: note: Assuming 'argv' is non-null
if (!argv)
^
exec.c:16:2: note: Taking false branch
if (!argv)
^
exec.c:21:2: note: Potential leak of memory pointed to by 'argv'
return execvp(file, argv);
---
I'm not used to thinking about allocations around exec, since usually
if it fails the program just terminates. But in a function like this,
the rules are a bit different.
exec.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/exec.c b/exec.c
index cc55200..75b1980 100644
--- a/exec.c
+++ b/exec.c
@@ -18,5 +18,8 @@ int execzp(const char *file, const char *argz, size_t len)
argz_extract(argz, len, argv);
- return execvp(file, argv);
+ execvp(file, argv);
+
+ free(argv);
+ return -1;
}
--
2.30.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH ucspi-vsock 2/2] vsockserver: fix uninitialized variable
2021-03-21 14:51 ` [PATCH ucspi-vsock 1/2] exec: free argv if exec fails Alyssa Ross
@ 2021-03-21 14:54 ` Alyssa Ross
0 siblings, 0 replies; 5+ messages in thread
From: Alyssa Ross @ 2021-03-21 14:54 UTC (permalink / raw)
To: devel; +Cc: Cole Helbling
Identified by clang-tidy:
vsockserver.c:105:17: warning: 3rd function call argument is an uninitialized value [clang-analyzer-core.CallAndMessage]
while ((opt = argz_next(binder_opts, binder_opts_len, opt)))
^
vsockserver.c:44:3: note: Assuming the condition is false
argz_add(&binder_opts, &binder_opts_len, BINDIR "/vsockserver-socketbinder") ||
^
vsockserver.c:44:3: note: Left side of '||' is false
vsockserver.c:47:9: note: Assuming the condition is false
while ((opt = getopt(argc, argv, "+1qQv")) != -1) {
^
vsockserver.c:47:2: note: Loop condition is false. Execution continues on line 68
while ((opt = getopt(argc, argv, "+1qQv")) != -1) {
^
vsockserver.c:68:6: note: Assuming 'alloc_failed' is false
if (alloc_failed)
^
vsockserver.c:68:2: note: Taking false branch
if (alloc_failed)
^
vsockserver.c:77:6: note: Assuming the condition is false
if (optind > argc - 3)
^
vsockserver.c:77:2: note: Taking false branch
if (optind > argc - 3)
^
vsockserver.c:81:6: note: Assuming the condition is false
if (argz_add(&binder_opts, &binder_opts_len, "--") ||
^
vsockserver.c:81:6: note: Left side of '||' is false
vsockserver.c:82:6: note: Assuming the condition is false
argz_add(&binder_opts, &binder_opts_len, argv[optind++]) ||
^
vsockserver.c:81:6: note: Left side of '||' is false
if (argz_add(&binder_opts, &binder_opts_len, "--") ||
^
vsockserver.c:83:6: note: Assuming the condition is false
argz_add(&binder_opts, &binder_opts_len, argv[optind++]))
^
vsockserver.c:81:2: note: Taking false branch
if (argz_add(&binder_opts, &binder_opts_len, "--") ||
^
vsockserver.c:89:6: note: Assuming the condition is false
if (argz_append(&binder_opts, &binder_opts_len, daemon_opts, daemon_opts_len))
^
vsockserver.c:89:2: note: Taking false branch
if (argz_append(&binder_opts, &binder_opts_len, daemon_opts, daemon_opts_len))
^
vsockserver.c:96:23: note: Assuming 'i' is >= 'argc'
for (int i = optind; i < argc; i++)
^
vsockserver.c:96:2: note: Loop condition is false. Execution continues on line 100
for (int i = optind; i < argc; i++)
^
vsockserver.c:100:6: note: Assuming 'verbosity' is equal to all
if (verbosity == all) {
^
vsockserver.c:100:2: note: Taking true branch
if (verbosity == all) {
^
vsockserver.c:101:3: note: 'opt' declared without an initial value
char *opt;
^
vsockserver.c:105:17: note: 3rd function call argument is an uninitialized value
while ((opt = argz_next(binder_opts, binder_opts_len, opt)))
---
vsockserver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/vsockserver.c b/vsockserver.c
index f740a8a..b717eee 100644
--- a/vsockserver.c
+++ b/vsockserver.c
@@ -98,7 +98,7 @@ int main(int argc, char *argv[])
diee(EX_OSERR, "malloc");
if (verbosity == all) {
- char *opt;
+ char *opt = 0;
// Log the full argv before we exec it.
fprintf(stderr, "%s: executing", program_invocation_short_name);
--
2.30.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH ucspi-vsock 0/2] Fix clang-tidy warnings
2021-03-21 14:46 [PATCH ucspi-vsock 0/2] Fix clang-tidy warnings Alyssa Ross
2021-03-21 14:51 ` [PATCH ucspi-vsock 1/2] exec: free argv if exec fails Alyssa Ross
@ 2021-03-21 16:30 ` Cole Helbling
2021-03-21 19:37 ` Alyssa Ross
1 sibling, 1 reply; 5+ messages in thread
From: Cole Helbling @ 2021-03-21 16:30 UTC (permalink / raw)
To: Alyssa Ross, devel; +Cc: Cole Helbling
On Sun Mar 21, 2021 at 7:46 AM PDT, Alyssa Ross wrote:
> clang-tidy also produced this warning:
>
> /home/src/ucspi-vsock/repro.c:43:19: warning: unused parameter 'sig' [clang-diagnostic-unused-parameter]
> void sig_exit(int sig) { exit(EX_UNAVAILABLE); }
> ^
> vsock.c:27:23: warning: The left operand of '!=' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
> if (addr->svm_family != AF_VSOCK) {
> ^
> vsock.c:90:6: note: Assuming the condition is false
> if (getsockname(fd, (struct sockaddr *)&addr, &addrlen) == -1)
> ^
> vsock.c:90:2: note: Taking false branch
> if (getsockname(fd, (struct sockaddr *)&addr, &addrlen) == -1)
> ^
> vsock.c:93:9: note: Calling 'fill_cid_and_port'
> return fill_cid_and_port(&addr, cid, port);
> ^
> vsock.c:27:23: note: The left operand of '!=' is a garbage value
> if (addr->svm_family != AF_VSOCK) {
>
> But I think this is just warning me that the POSIX socket API violates
> the strict aliasing rule. (Which is true, but there's not a lot I can
> do about it...)
>
> It also tells me I should use memset_s instead of memset, but, well...
> https://en.wikipedia.org/wiki/C11_(C_standard_revision)#Criticism
>
> Alyssa Ross (2):
> exec: free argv if exec fails
> vsockserver: fix uninitialized variable
>
> exec.c | 5 ++++-
> vsockserver.c | 2 +-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> --
> 2.30.0
Patchset LGTM.
Reviewed-by: Cole Helbling <cole.e.helbling@outlook.com>
(I should really make a macro or something for that ^, so I don't have
to type it out every time and potentially make a mistake :D)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH ucspi-vsock 0/2] Fix clang-tidy warnings
2021-03-21 16:30 ` [PATCH ucspi-vsock 0/2] Fix clang-tidy warnings Cole Helbling
@ 2021-03-21 19:37 ` Alyssa Ross
0 siblings, 0 replies; 5+ messages in thread
From: Alyssa Ross @ 2021-03-21 19:37 UTC (permalink / raw)
To: Cole Helbling; +Cc: devel
[-- Attachment #1: Type: text/plain, Size: 2036 bytes --]
On Sun, Mar 21, 2021 at 09:30:42AM -0700, Cole Helbling wrote:
> On Sun Mar 21, 2021 at 7:46 AM PDT, Alyssa Ross wrote:
> > clang-tidy also produced this warning:
> >
> > /home/src/ucspi-vsock/repro.c:43:19: warning: unused parameter 'sig' [clang-diagnostic-unused-parameter]
> > void sig_exit(int sig) { exit(EX_UNAVAILABLE); }
> > ^
> > vsock.c:27:23: warning: The left operand of '!=' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
> > if (addr->svm_family != AF_VSOCK) {
> > ^
> > vsock.c:90:6: note: Assuming the condition is false
> > if (getsockname(fd, (struct sockaddr *)&addr, &addrlen) == -1)
> > ^
> > vsock.c:90:2: note: Taking false branch
> > if (getsockname(fd, (struct sockaddr *)&addr, &addrlen) == -1)
> > ^
> > vsock.c:93:9: note: Calling 'fill_cid_and_port'
> > return fill_cid_and_port(&addr, cid, port);
> > ^
> > vsock.c:27:23: note: The left operand of '!=' is a garbage value
> > if (addr->svm_family != AF_VSOCK) {
> >
> > But I think this is just warning me that the POSIX socket API violates
> > the strict aliasing rule. (Which is true, but there's not a lot I can
> > do about it...)
> >
> > It also tells me I should use memset_s instead of memset, but, well...
> > https://en.wikipedia.org/wiki/C11_(C_standard_revision)#Criticism
> >
> > Alyssa Ross (2):
> > exec: free argv if exec fails
> > vsockserver: fix uninitialized variable
> >
> > exec.c | 5 ++++-
> > vsockserver.c | 2 +-
> > 2 files changed, 5 insertions(+), 2 deletions(-)
> >
> > --
> > 2.30.0
>
> Patchset LGTM.
>
> Reviewed-by: Cole Helbling <cole.e.helbling@outlook.com>
Thanks!
To ssh://atuin/home/spectrum/git/ucspi-vsock.git
20a27f1..8b690b9 8b690b9 -> master
> (I should really make a macro or something for that ^, so I don't have
> to type it out every time and potentially make a mistake :D)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-03-21 19:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-21 14:46 [PATCH ucspi-vsock 0/2] Fix clang-tidy warnings Alyssa Ross
2021-03-21 14:51 ` [PATCH ucspi-vsock 1/2] exec: free argv if exec fails Alyssa Ross
2021-03-21 14:54 ` [PATCH ucspi-vsock 2/2] vsockserver: fix uninitialized variable Alyssa Ross
2021-03-21 16:30 ` [PATCH ucspi-vsock 0/2] Fix clang-tidy warnings Cole Helbling
2021-03-21 19:37 ` Alyssa Ross
Code repositories for project(s) associated with this public inbox
https://spectrum-os.org/git/crosvm
https://spectrum-os.org/git/doc
https://spectrum-os.org/git/mktuntap
https://spectrum-os.org/git/nixpkgs
https://spectrum-os.org/git/spectrum
https://spectrum-os.org/git/ucspi-vsock
https://spectrum-os.org/git/www
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).