[PATCH] D25876: [analyzer] Report CFNumberGetValue API misuse

[Devin Coughlin via cfe-commits]

dcoughlin added inline comments.


================
Comment at: test/Analysis/CFNumber.c:39
+  unsigned char scalar = 0;
+  CFNumberGetValue(x, kCFNumberSInt16Type, &scalar);  // expected-warning{{A CFNumber object that represents a 16 bit integer is used to initialize an 8 bit integer. 8 bits of the CFNumber value will be lost}}
+  return scalar;
----------------
I feel like this diagnostic is not dire enough. The problem is not that the bits will be lost -- but rather that they will be overwritten on top of something else!

How about something like "The remaining 8 bits will overwrite adjacent storage."?


================
Comment at: test/Analysis/CFNumber.c:45
+  unsigned short scalar = 0;
+  CFNumberGetValue(x, kCFNumberSInt8Type, &scalar);  // expected-warning{{A CFNumber object that represents an 8 bit integer is used to initialize a 16 bit integer. 8 bits of the integer value will be garbage}}
+  return scalar;
----------------
Some grammar/style nits. (I realize these also hold for the pre-existing checks):

- There should be a dash between the number and 'bit'. That is, it should be "16-bit" and "8-bit".
- It is generally considered bad style to start a sentence with a number that is not written out (except for dates). How about starting the second sentence with "The remaining 8 bits ..."?


https://reviews.llvm.org/D25876