Re: Return type of clean_and_invalidate_dcache_va_range

[Julien Grall <julien@xxxxxxx>]

Hi,

On 09/02/2024 22:02, Stefano Stabellini wrote:
> On Fri, 9 Feb 2024, Nicola Vetrini wrote:
> > Hi all,
> >
> > In the context of violations of MISRA C:2012 Rule 17.7: "The value returned by
> > a function having non-void return type shall be used", I was looking at the
> > function "clean_and_invalidate_dcache_va_range". It has the following
> > signature on both arm and x86:
> >
> > static inline int clean_and_invalidate_dcache_va_range
> >      (const void *p, unsigned long size)
> >
> > The commit that introduced it for Arm ~9 years ago (71d64afe3e12: "arm: return
> > int from *_dcache_va_range") [1] mentions that on Arm it can't fail, but
> > supposedly it can on x86.
> >
> > However, as far as I can tell, for both arch-es the implementation now always
> > returns 0 [2][3], so perhaps the mention of -EOPNOTSUPP for x86 is no longer
> > true (I wasn't able to reconstruct if there was a time at which this was true,
> > even in the same commit that changed the return type to int).
> >
> > The question is: should the return type be void, since it appears that every
> > user is ignoring the returned value (violating the rule), except the one in
> > common/grant_table.c [4]?
> Looking at the implementation on both ARM and x86, I am in favor of
> changing the return type to void
I think we need some consistency between all the cache flush helpers 
(clean_and_invalidate_dcache_va_range, invalidate_dcache_va_range() and 
clean_dcache_va_range()). They should all return a values or not return any.
That said, we have two other architectures in development. Are we saying 
this helpers will not need to (initially) return -EOPNOTSUPP?
If we need to return a value for any of the cache helpers, then I would 
consider to provide an extra helpers that would only be used by the 
grant-table code and return a value. Then transform all the existing 
helpers to return void.
Cheers,

--
Julien Grall