On 1 July 2016 at 14:08, Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
A few suggestions:
Make the function take separate src and dst parameters, making it explicitly allowed to pass the same value (but not other kinds of overlap, of course). That way one can avoid "strcpy(dst, src); strtolower(dst);".
Drop the NULL check. If someone does "foo->bar = something; strtolower(foo->bar); put foo in a global data structure...", the dereference of foo->bar may happen much later. Doing the NULL deref sooner means it's much easier to find and fix the bug. (Also, other str* and mem* functions don't usually check for NULL).
While it's true that strcpy and memcpy by definition return dst, that's mostly useless. If you want it to return anything, please make it something that might be used - for example, having stpcpy semantics (returning a pointer to dst's terminating \0) means a caller might avoid a strlen call.
Maybe do strtoupper while you're at it. Quick grepping didn't find any use for the copy-while-lowercasing, but copy-while-uppercasing can at least be used in drivers/acpi/acpica/nsrepair2.c, drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c, drivers/power/power_supply_sysfs.c along with a bunch of inplace uppercasing.
Rasmus
Thanks for the suggestions to you and Jani. Based on the feedback I received, I am reworking the series now and will post v2 probably tomorrow.
Regards, -Markus