FIX: fix sanitizer warnings #49
Conversation
|
Actually, I've decided that I'm going to add clang-tidy in CI. Going to mark this as draft until I've done that. |
JJL772
force-pushed
the
fix_sanitizer_warnings
branch
11 times, most recently
from
67d43da to
84e9ee6
Compare
|
CI seems to be functional now. |
JJL772
force-pushed
the
fix_sanitizer_warnings
branch
2 times, most recently
from
5daa099 to
bd8609c
Compare
clang-tidy.py
Outdated
There was a problem hiding this comment.
Is this a normal way of doing things?
Based on my experience, I think pre-commit and its exclusion regex would make this pretty easy (and not require custom Python code/JSON).
https://pre-commit.com/#regular-expressions
There was a problem hiding this comment.
I'm going to have clang-format as a pre-commit hook, but not clang-tidy. There's a bunch of setup required for clang-tidy like generating compile_commands.json. It also takes about a minute to run.
| @@ -204,8 +204,8 @@ devEK9000Terminal* devEK9000Terminal::ProcessRecordName(const char* recname, int | |||
| for (size_t i = len; i >= 0; i--) { | |||
| if (ret[i] == ':' && (size_t)i < len) { | |||
| ret[i] = '\0'; | |||
| good = true; | |||
| *outindex = atoi(&ret[i + 1]); | |||
| if (epicsParseInt32(ret + 1, outindex, 10, NULL) == 0) | |||
There was a problem hiding this comment.
Was this where your parseInt utility should have come in?
There was a problem hiding this comment.
Yep, epicsParseInt32 works fine though. The return value is a bit weird so I was trying to avoid the footgun with parseInt.
|
Small set of merge conflicts need to be resolved here, it appears |
JJL772
force-pushed
the
fix_sanitizer_warnings
branch
from
bd8609c to
dbade0d
Compare
the usage field was a relatively new addition
JJL772
force-pushed
the
fix_sanitizer_warnings
branch
2 times, most recently
from
a9f58a8 to
bce5a3d
Compare
|
Sorry for the mess. Was trying to add pre-commit here, but ended up hitting some weird issues with the clang-format CI check. Will be done in a separate PR instead. Also added a quick build fix for older versions of EPICS base, needed for our base 7.0.2 builds. |
|
Tests passing and no more merge conflict. LGTM - merge at will 👍 |
Fixes some warnings reported by clang-tidy. None of this code is being used currently (will be eventually), but it's good to fix at least.