* [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).