Dave, Russell, On Mon, Jan 17, 2011 at 4:46 PM, Dave Martin <dave.martin@xxxxxxxxxx> wrote: > On Mon, Jan 17, 2011 at 2:01 PM, Jean Pihet <jean.pihet@xxxxxxxxxxxxxx> wrote: >> On Fri, Jan 14, 2011 at 6:34 PM, Russell King - ARM Linux >> <linux@xxxxxxxxxxxxxxxx> 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. > Ok I reworked the patch from your suggestions. Indeed a few functions types mismatch have been spotted and corrected using the fncpy API. New patch sent as '[PATCH v2] OMAP: use fncpy to copy the PM code functions to SRAM'. > > Cheers > ---Dave > Thanks, Jean -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
