From: Kees Cook

Sent: 04 May 2022 16:38

...

quoted

quoted

    struct something *instance = NULL;
    int rc;

    rc = mem_to_flex_dup(&instance, byte_array, count, GFP_KERNEL);
    if (rc)
        return rc;

This seems rather awkward, having to set it to NULL, then checking rc
(and possibly needing a separate variable for it), etc.

I think the errno return is completely required. I had an earlier version
of this that was much more like a drop-in replacement for memcpy that
would just truncate or panic, and when I had it all together, I could
just imagine hearing Linus telling me to start over because it was unsafe
(truncation may be just as bad as overflow) and disruptive ("never BUG"),
and that it should be recoverable. So, I rewrote it all to return a
__must_check errno.

Requiring instance to be NULL is debatable, but I feel pretty strongly
about it because it does handle a class of mistakes (resource leaks),
and it's not much of a burden to require a known-good starting state.

Why not make it look like malloc() since it seems to be malloc().
That gives a much better calling convention.
Passing pointers and integers by reference can generate horrid code.
(Mostly because it stops the compiler keeping values in registers.)

If you want the type information inside the 'function'
use a #define so that the use is:

	mem_to_flex_dup(instance, byte_array, count, GFP_KERNEL);
	if (!instance)
		return ...
(or use ERR_PTR() etc).

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)