The general approach here was:
- Always declare variables as close to where they are defined as
possible.
- Check for pre-conditions of functions before doing work (e.g. ensure
we can connect to the DB before doing a bunch of string formatting)
- Keep the scope of mutexes as reasonably small as practical.
- Use idiomatic C11, such as for-loops over the thing being iterated,
not while() loops over constants, or variables that aren't modified.
- Prefer if(fail){return} function-body after over `if(not fail){
function-body inside if} return;
Clang-tidy returns a clean bill of health, but while going through this
file i noticed a lot of things that raise questions.
Lack of checking column counts. Lack of handling the possibility of
multiple return values. Questionably handling of strings. Complete lack
of checking function inputs for invalid values (e.g. nullptr).
I'm not going to fix those, my organization doesn't USE the DB drivers,
so i have little interest in re-working the logic beyond addressing
clang-tidy warnings for my own sanity, but i did add TODO comments for
someone else to look at in the future.
Additional note: While the changes look very invasive.... they aren't.
I don't think there is a way to get github to ignore whitespace in the
filediff, but if someone were to compare the commit locally, they'll see
that almost all of the changes are just adjusting indentation.
Converts all of the variables in the uclient program that should be bool
but weren't.
A few other minor adjustments made at the behest of clang-tidy, but this
change does not address all of clang-tidy's complaints.
I'm having trouble testing changes in my own fork because many of the CI
workflows are only set to build on a pull request.
Better to just build on any commit.
I've left the docker builds as they are.
#1502 made APIs consistent with using bool as a return value where true
is success and false is failure
In a few places the change broke code
This PR fixes the breakage
The code used `get_system_number_of_cpus()` instead of
`get_system_active_number_of_cpus()` to configure number of relay
servers.
That caused incorrect number to be used on virtualized systems. See
#1468
Someone working on a branch in their own fork won't see the lint running
on their github-actions CI until they open a pull request.
It'd be much easier to ensure changes being worked on are going to pass
the linter if the linter runs before the last step of opening the PR.
rfc5769check.c file is using ERROR as a label for gotos but apparently
that name is already used for a constant and msvc analyzer detects it as
an error.
Rename it to "err" that is already used in other parts of the codebase
and also more consistent in terms of casing.
Co-authored-by: Gustavo Garcia <gustavogb@mail.com>
Fixed an issue in libevent's CMake configuration where pthreads were not
correctly added to the optional components list, leading to a
compilation error. #1448
Co-authored-by: linwenchen <wenchen0803@qq.com>
I came across the open issue #1368 which is a leaksanitizer report about
a leak that happens on make check. I was able to reproduce the bug on
running `make check` and was able to generate a patch for it. The leak
happens within function `check_oauth`.
Issue: The variable `base64encoded_ltp` is allocated within
`base64_encode` however it is not de-allocated within the coming loop.
I have verified that after the patch, the leak no longer occurs through
`leaksanitizer` (and there is no double free for that matter)
---------
Co-authored-by: Pavel Punsky <eakraly@users.noreply.github.com>
This is in response to issue #1366
The clang static analyzer basically claims that there is a memory leak
happening in `set_ssl_ctx` for the variable `args`. The leak is
triggered when the base event `base` is NULL and the condition within
`set_ssl_ctx` is not triggered. Therefore as a patch I am adding an else
condition to free it. (It cannot be freed after the event is created by
`event_new` because `args` can be invoked as argument for callback
function later on)
Please let me know if this patch is helpful :)
---------
Co-authored-by: Pavel Punsky <eakraly@users.noreply.github.com>
This is in response to issue #1365.
The clang static analyzer basically claims that there is a memory leak
happening in `parse_http_request_1` for the variable `kv`. The leak is
triggered when evhttp_parse_query_str fails and is unable to obtain key
value pairs for a given URI. In this case ret is freed, however kv is
still not freed and thereafter not used. Therefore as a patch I am
freeing kv right after ret is freed.
Please let me know if this patch is helpful :)
---------
Co-authored-by: Pavel Punsky <eakraly@users.noreply.github.com>
# changed variables that appeared in `stunclient.c` and their uses to
`bool` to follow C11 idioms
## approach was as follows:
- if a variable of type `int` was only being used as a boolean, replace
it with bool as defined in `<stdbool.h>`
- replace its declaration and assignment with true/false, depending on
prior assignment as 0/1
changes were only made when i was certain the variables were not being
used as an int, so i may have missed some
---
## variables changed in `stunclient.c`
- `rfc5780`
- `change_ip`
- `change_port`
their usages were changed only where they appeared in the apps
directory, and then everywhere that generated a warning after make
- `stunclient.c` itself
- `natdiscovery.c`
trivial - cmake generated a warning that `ns_turn_msg.c` used a function
that had no prior prototype - most other files used the `turn_random()`
function but this uses `turn_random_number()` which has no prototype, so
i've added it to the header file
Michael Jones
redraincatching
Pavel Punsky
tyranron
Gustavo Garcia
Stefan Junker
Nikolayshcx
Gustavo Garcia
Kang Lin
Wittmer, Christian
NO NAME
dependabot[bot]
ashamedbit
Jan Brasna
dominiquefournier