On Wed, Aug 25, 2021 at 7:47 PM Kees Cook <keescook@xxxxxxxxxxxx> wrote: > > On Wed, Aug 25, 2021 at 02:48:30PM -0700, Nick Desaulniers wrote: > > are memset16, memset32, and memset64 worth moving as well? Also, > > memscan(), check_bytes(), memchr_inv()? > > All of these are implementations, so they should stay put. All of the functions being moved here are definitions. So what's the difference between moving the definitions of functions like strrreplace, fortify_panic, etc., but not memscan(), check_bytes(), memchr_inv(), etc? ie. it looks to me like a few more functions can or should be moved as well. If the point of this patch is to "move all the helper functions into string_helpers.c so that they gain the fortification coverage they had been missing" then it looks like you missed a few. I don't think the compiler will recognize those non-libc identifiers for any fortification related transforms (unlike memcpy and friends which are left in place). -- Thanks, ~Nick Desaulniers