[PATCH] OMAP: use fncpy to copy the PM code functions to SRAM

[Dave Martin]

On Mon, Jan 17, 2011 at 2:01 PM, Jean Pihet <jean.pihet at newoldbits.com> wrote:
> On Fri, Jan 14, 2011 at 6:34 PM, Russell King - ARM Linux
> <linux at arm.linux.org.uk> wrote:
>> On Fri, Jan 14, 2011 at 05:13:01PM +0100, Jean Pihet wrote:
>>> Is the name 'omap_sram_push' wrong then?
>>> What about the following?
>>> @@ -251,9 +251,8 @@ void * omap_sram_push(void * start, unsigned long size)
>>>
>>>        omap_sram_ceil -= size;
>>>        omap_sram_ceil = ROUND_DOWN(omap_sram_ceil, sizeof(void *));
>>>  -     memcpy((void *)omap_sram_ceil, start, size);
>>>  -     flush_icache_range((unsigned long)omap_sram_ceil,
>>>  -             (unsigned long)(omap_sram_ceil + size));
>>>
>>>  -     return (void *)omap_sram_ceil;
>>>  +    return fncpy((void *)omap_sram_ceil, start, size);
>>
>> It's more correct, but still missing out on the type safety which we've
>> tried to provide with fncpy.
> IIUC the type of the function is propagated from the 2nd argument
> (funcp) to the return value, which is fine here. The (void)* is here
> only to avoid a warning thrown by memcpy.

I think Russell's argument was that the compiler will not notice if
you mismatch the return type and the function to be copied, because
the casts to/from void * make the compiler blind to the real types
involved.  So:

int f(int);
size_t size_of_f;

void *buffer;
int (*_copied_f1)(int);
char *(*_copied_f2)(char *);

_copied_f1 = fncpy(buffer, f, size_of_f); /* OK */
_copied_f2 = fncpy(buffer, f, size_of_f); /* compile error */
_copied_f1 = omap_sram_push(f, size_of_f); /* OK */
_copied_f2 = opam_sram_push(f, size_of_f); /* type mismatch, but
compilation succeeds */


One way to work around this is would be to make omap_sram_push() a macro:

#define omap_sram_push(funcp, size) \
    (typeof(funcp))_do_omap_sram_push((void *)(funcp), size)

... where the definition of _do_omap_sram_push() is the same is the
existing definition of omap_sram_push().  Providing
_do_omap_sram_push() is not called directly, this should now be
type-safe.


Cheers
---Dave