[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check

Fangrui Song via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 14 14:50:56 PDT 2018


MaskRay added inline comments.


================
Comment at: test/Analysis/cstring-syntax.c:49
+  strlcat(dest, "0123456789", badlen / 2);
+  strlcat(dest, "0123456789", badlen); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value 'badlen' - strlen(dest) - 1 or lower}}
+  strlcat(dest, "0123456789", badlen - strlen(dest) - 1);
----------------
devnexen wrote:
> NoQ wrote:
> > The suggested fix is a bit weird.
> > 
> > The correct code for appending `src` to `dst` is either `strlcat(dst, src, sizeof(dst));` (the approach suggested by the man page) or `strlcat(dst + strlen(dst) + 1, src, sizeof(dst) - strlen(dst) - 1)` (which is equivalent but faster if you already know `strlen(dst)`). In both cases you can specify a smaller value but not a larger value.
> In fact in this case the message is misleading/a bit wrong.
I think `strlcat` is very clumsy to you if you need to add an offset to `dest`...

For
`strlcat(dst + strlen(dst) + 1, src, sizeof(dst) - strlen(dst) - 1)`

I suppose you meant:

`strlcpy(dst + strlen(dst), src, sizeof(dst) - strlen(dst))`

... but the suggestion does not look very appealing. `strlcat(dst, ..., sizeof(dst)` if good enough as a suggested fix.


https://reviews.llvm.org/D49722





More information about the cfe-commits mailing list