[PATCH] D130470: [clang][analyzer] Add more wide-character functions to CStringChecker

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: rnkovacs.

In D130470#3682131 <https://reviews.llvm.org/D130470#3682131>, @balazske wrote:

> A related question:
> Is it better to have a more generic function like this:
>
>   void CStringChecker::evalMemmove(CheckerContext &C, const CallExpr *CE,
>                                    bool IsRestricted, bool IsMempcpy, bool IsWide) const {
>     // void *memmove(void *dst, const void *src, size_t n);
>     // The return value is the address of the destination buffer.
>     DestinationArgExpr Dest = {CE->getArg(0), 0};
>     SourceArgExpr Src = {CE->getArg(1), 1};
>     SizeArgExpr Size = {CE->getArg(2), 2};
>   
>     evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy,
>                    IsWide);
>   }
>
> (Instead of `bool` enums can be used.) Or make additional versions of the eval functions and remove the `IsWide` argument (this is even more code repetition)?

I'd strive for the least code repetition. But I consider this just some nice to have sugar.

LGTM



================
Comment at: clang/test/Analysis/wstring.c:372-376
+  // else
+  //   analyzer_assert_unknown(n == 0);
+
+  // We can't do the above comparison because n has already been constrained.
+  // On one path n == 0, on the other n != 0.
----------------
This comment is probably outdated (I know it has been copied from `bstring.c` but still). Perhaps we should have the `else` branch now not just here but also in `bstring.c`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130470