From: Kees Cook
> Sent: 04 May 2022 16:38
...
> > > 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)
