On Wed, Sep 01, 2021 at 09:24:12PM +0200, Mattias Rönnblom wrote:
> On 2021-09-01 18:32, Bruce Richardson wrote:
> > Run basic sanity tests for configuring, starting and stopping a dmadev
> > instance to help validate drivers. This also provides the framework for
> > future tests for data-path operation.
> >
> > Signed-off-by: Bruce Richardson <bruce.richard...@intel.com>
> > ---
> > app/test/test_dmadev.c | 81 ++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 81 insertions(+)
> >
> > diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c
> > index bb01e86483..12f7c69629 100644
> > --- a/app/test/test_dmadev.c
> > +++ b/app/test/test_dmadev.c
> > @@ -2,6 +2,7 @@
> > * Copyright(c) 2021 HiSilicon Limited.
> > * Copyright(c) 2021 Intel Corporation.
> > */
> > +#include <inttypes.h>
> > #include <rte_common.h>
> > #include <rte_dev.h>
> > @@ -13,6 +14,77 @@
> > /* from test_dmadev_api.c */
> > extern int test_dmadev_api(uint16_t dev_id);
> > +#define PRINT_ERR(...) print_err(__func__, __LINE__, __VA_ARGS__)
> > +
> > +static inline int
>
> Remove inline.
>
While I understand it's probably not doing a lot having "inline" there, any
particular reason why you think it should be removed?
> > +__rte_format_printf(3, 4)
> > +print_err(const char *func, int lineno, const char *format, ...)
> > +{
> > + va_list ap;
> > + int ret;
> > +
> > + ret = fprintf(stderr, "In %s:%d - ", func, lineno);
> Check return code here, and return on error.
>
> > + va_start(ap, format);
> > + ret += vfprintf(stderr, format, ap);
>
> ..and here.
>
> > + va_end(ap);
> > +
> > + return ret;
>
> A negative return value in one call and an valid byte count result for the
> other should produce an error, but here it might not.
>
> You might argue this is just test code, but then I suggest not checking the
> return values at all.
>
Indeed the return value is never checked anywhere in the calls to PRINT_ERR
macro, and since the writes are going to stderr it's pretty low risk.
Therefore, I'll remove the return value handling completely as you suggest.
/Bruce
