[PATCH] D18783: [clang-tidy] add new checker for string literal with NUL character.
Etienne Bergeron via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 6 09:49:18 PDT 2016
etienneb added a comment.
In http://reviews.llvm.org/D18783#393367, @LegalizeAdulthood wrote:
> There are some APIs where a series of strings are passed as a single `char *` with NUL separators between the strings and the last string is indicated by two NUL characters in a row. For instance, see the MSDN documentation <https://msdn.microsoft.com/en-us/library/windows/desktop/ms682450(v=vs.85).aspx> for the `lpDependencies` argument to `CreateService` <https://msdn.microsoft.com/en-us/library/windows/desktop/ms682450(v=vs.85).aspx>. Such strings literals in the code look like:
> LPCTSTR dependencies = _T("one\0two\0three\0four\0");
> I don't see any annotation on this argument in `<winsvc.h>` where this function is declared, so there's no hint from the header about this argument. In this case, the biggest mistake people make is omitting the final NUL character, thinking that the `\0` acts as a string separator and not a terminator:
> LPCTSTR dependencies = _T("one\0two\0three\0four");
> If you're lucky this results in a crash right away. If you're unlucky the compiler placed this constant in an area of memory with zero padding and it works by accident on your machine until it ships to a customer and then breaks on their machine mysteriously.
> I was never a fan of these magic string array arguments and they mostly appear in the older portions of the Win32 API, but they are there nonetheless. I would hate for this check to signal a bunch of false positives on such strings.
I know about these kind functions, and they are not covered by the current state of this check.
The current check only apply if there is a conversion from char* -> std::string (implicit constructor).
For now, I want to keep the false-positive ratio low.
I made a prototype to detect instances passed to 'func(char*)' but there are too many false-positives.
More information about the cfe-commits