[cfe-commits] r137980 - in /cfe/trunk: include/clang/Basic/Builtins.def include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaChecking.cpp test/Sema/warn-strlcpycat-size.c

[Eli Friedman]

On Thu, Aug 18, 2011 at 2:42 PM, Ted Kremenek <kremenek at apple.com> wrote:
>
> On Aug 18, 2011, at 2:33 PM, Eli Friedman wrote:
>
> +  // Output a FIXIT hint if the destination is an array (rather than a
>
> +  // pointer to an array).  This could be enhanced to handle some
>
> +  // pointers if we know the actual size, like if DstArg is 'array+2'
>
> +  // we could say 'sizeof(array)-2'.
>
> +  const Expr *DstArg = Call->getArg(0)->IgnoreParenImpCasts();
>
> +
>
> +  if (DstArg->getType()->isArrayType()) {
>
> +    llvm::SmallString<128> sizeString;
>
> +    llvm::raw_svector_ostream OS(sizeString);
>
> +    OS << "sizeof(";
>
> +    DstArg->printPretty(OS, Context, 0, Context.PrintingPolicy);
>
> +    OS << ")";
>
> +
>
> +    Diag(OriginalSizeArg->getLocStart(), diag::note_strlcpycat_wrong_size)
>
> +      << FixItHint::CreateReplacement(OriginalSizeArg->getSourceRange(),
>
> +                                      OS.str());
>
> +  }
>
> +}
>
> I would say this fixit isn't a good idea, both because we should be
> forcing the programmer to think about what they are doing here, and
> because it isn't necessarily correct.  (sizeof(dest) isn't guaranteed
> to be the actual size of the destination; suppose, for example, dest
> is a flexible array.)
>
> 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.
> 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.

Hmm, okay.  (Don't forget to suppress the fixit for arrays of length 1.)

-Eli