[PATCH] D70411: [analyzer] CERT: STR31-C

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

Thanks for the review, hopefully if I ping @NoQ in every round, it will be green-marked soon.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:55
+  // they can cause a not null-terminated string. In this case we store the
+  // string is being not null-terminated in the 'NullTerminationMap'.
+  //
----------------
balazske wrote:
> The functions `gets`, `strcpy` return a null-terminated string according to standard, probably `sprintf` too, and the `fscanf` `%s` output is null-terminated too even if no length is specified (maybe it writes outside of the specified buffer but null terminated).
>From where do you observe such information? I have not seen anything `strcpy()`-specific in the standard. My observation clearly says that, if there is not enough space for the data you *could* even write to overlapping memory, or as people says, you could meet nasal demons. It is called *undefined* behavior.


================
Comment at: clang/test/Analysis/cert/str31-c.cpp:117
+  if (editor != NULL) {
+    size_t len = strlen(editor) + 1;
+    buff2 = (char *)malloc(len);
----------------
balazske wrote:
> Do we get a warning if the `+1` above is missing?
Test added.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70411/new/

https://reviews.llvm.org/D70411