<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Aug 18, 2011, at 2:33 PM, Eli Friedman wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><span class="Apple-style-span" style="border-collapse: separate; font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; font-size: medium; "><div><blockquote type="cite">+ // Output a FIXIT hint if the destination is an array (rather than a<br></blockquote><blockquote type="cite">+ // pointer to an array). This could be enhanced to handle some<br></blockquote><blockquote type="cite">+ // pointers if we know the actual size, like if DstArg is 'array+2'<br></blockquote><blockquote type="cite">+ // we could say 'sizeof(array)-2'.<br></blockquote><blockquote type="cite">+ const Expr *DstArg = Call->getArg(0)->IgnoreParenImpCasts();<br></blockquote><blockquote type="cite">+<br></blockquote><blockquote type="cite">+ if (DstArg->getType()->isArrayType()) {<br></blockquote><blockquote type="cite">+ llvm::SmallString<128> sizeString;<br></blockquote><blockquote type="cite">+ llvm::raw_svector_ostream OS(sizeString);<br></blockquote><blockquote type="cite">+ OS << "sizeof(";<br></blockquote><blockquote type="cite">+ DstArg->printPretty(OS, Context, 0, Context.PrintingPolicy);<br></blockquote><blockquote type="cite">+ OS << ")";<br></blockquote><blockquote type="cite">+<br></blockquote><blockquote type="cite">+ Diag(OriginalSizeArg->getLocStart(), diag::note_strlcpycat_wrong_size)<br></blockquote><blockquote type="cite">+ << FixItHint::CreateReplacement(OriginalSizeArg->getSourceRange(),<br></blockquote><blockquote type="cite">+ OS.str());<br></blockquote><blockquote type="cite">+ }<br></blockquote><blockquote type="cite">+}<br></blockquote><br>I would say this fixit isn't a good idea, both because we should be<br>forcing the programmer to think about what they are doing here, and<br>because it isn't necessarily correct. (sizeof(dest) isn't guaranteed<br>to be the actual size of the destination; suppose, for example, dest<br>is a flexible array.)</div></span></blockquote></div><br><div>If the array is of constant size, or even a VLA, I don't see any problem with emitting a FIXIT. You're right that a FIXIT shouldn't be emitted for flexible arrays.</div><div><br></div><div>As for whether or not we should issue a FIXIT, I think we do more harm than good if we can emit a FIXIT that is 99% likely to be okay. The problem we are trying to solve is that people use these APIs incorrectly all the time. Where we are absolutely confident that we can correct the problem, I don't see the harm in doing so. When you say "think about what they are doing here", the problem is that often the programmer doesn't know what they should be doing. Sometimes showing them how to fix the problem goes a long way to educating them.</div></body></html>