Re: [PATCH 3/3] selftests: Add printf attribute to ksefltest prints

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2313 bytes --]

On Fri, 25 Aug 2023, Maciej Wieczór-Retman wrote:

> On 2023-08-25 at 11:28:17 +0300, Ilpo Järvinen wrote:
> >On Fri, 25 Aug 2023, Maciej Wieczór-Retman wrote:
> >> On 2023-08-24 at 16:10:12 +0300, Ilpo Järvinen wrote:
> >> >On Thu, 24 Aug 2023, Wieczor-Retman, Maciej wrote:
> >> >
> >> >> Kselftest header defines multiple variadic function that use printf
> >> >> along with other logic
> >> >> 
> >> >> There is no format checking for the variadic functions that use
> >> >> printing inside kselftest.h. Because of this the compiler won't
> >> >> be able to catch instances of mismatched print formats and debugging
> >> >> tests might be more difficult
> >> >> 
> >> >> Add the common __printf attribute macro to kselftest.h
> >> >> 
> >> >> Add __printf attribute to every function using formatted printing with
> >> >> variadic arguments
> >> >
> >> >Please add . to terminate the sentences.
> >> 
> >> Thanks, I'll fix it in the next version
> >> 
> >> >The patch looks fine:
> >> >Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> >> >
> >> >...However, there are formatting errors it found yet to fix.
> >> 
> >> I believe you mean cache.c#L297.
> >> 
> >> I think I saw you're preparing some patches that remove the line that
> >> reports the formatting error so I chose to not correct here.
> >> 
> >> Please let me know if I still should change it or would that be
> >> redundant.
> >
> >There are other selftests besides resctrl which had a few warnings.
> >
> >Making the selftests to rebuild though might be a bit tricky (you won't 
> >see the warnings otherwise), I don't know the command needed to clean 
> >selftests but I guess one can always force their timestamps to force 
> >recompile with:
> >  git ls-files tools/testing/selftests | xargs touch
> 
> Okay, I think I counted 13 more caused by the __printf().
> I just ran:
> 	$ make -C tools/testing/selftests
> and to rerun it later:
> 	$ make -C tools/testing/selftests clean
> 
> But do you think all these fixes fit into this series?
> Is so, should I put them in a separate patch or just append to this one?

Please fix them in separate patches.

IMO, the most logical approach is to make own series out of these printf 
attribute fixes and change, and another series for anything that is purely 
resctrl related.

-- 
 i.