[PATCH] D30771: [analyzer] Teach the MallocChecker about Glib API for two arguments

[Leslie Zhai via Phabricator via cfe-commits]

xiangzhai updated this revision to Diff 95874.
xiangzhai added a comment.

Hi David,

Thanks for your review!

I updated my patch as you suggested! Please review it, thanks a lot! but I argue that:

> I have the feeling this should be renamed. Since its purpose is to calculate the total size maybe MallocChecker::calculateBufferSize()

Yes! it is difficult to suitably name variable, function, project, and even my kid :) but `calculateBufferSize` is not make sense:

1. how to calculate? by `evalBinOp` do `BO_Mul` operation.
2. for what? for bytes size <https://github.com/bminor/glibc/blob/master/malloc/malloc.c#L3183> so `forBufferSize` is ok.

> Capital variable: arg0Expr, arg0Val

I hold the view that I need to respect original developers' code, and it need a Global Patch for Capital variable, just like KDE's Use nullptr everywhere <https://phabricator.kde.org/D3987>

> is this "if (!Arg1)" needed? It seems to me that if CE->getNumArgs() returns >= 2 then CE->getArg(1) should be non-NULL. Does it crash/assert if you remove this condition?

The same story.

Regards,
Leslie Zhai


Repository:
  rL LLVM

https://reviews.llvm.org/D30771

Files:
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  test/Analysis/gmalloc.c

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D30771.95874.patch
Type: text/x-patch
Size: 15401 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170420/1e52cf8d/attachment-0001.bin>