Re: [Accel-config] [PATCH v2] accel-config: Fixes for pedantic compiler warnings

On 8/24/23 18:37, Ramesh Thomas wrote:
> Debian builds enable the -Wpedantic option that gives warnings for
> features that are supported by GCC but prohibited by other compilers.
> This project is tied to the GCC compiler and uses several GCC
> builti-ins. Provide fixes to warnings that are potential issues.
> Several benighn warnings are suppressed using GCC #pragma.
> 
> Signed-off-by: Ramesh Thomas <[email protected]>

Reviewed-by: Dave Jiang <[email protected]>
> ---
> v2:
> Fixed xrealloc() to call die() if realloc fails, without retrying with
> size==1
> 
>   accfg/idxd.h         | 42 ++++++++++++++++++++----------------------
>   accfg/lib/libaccfg.c | 20 +++++++++-----------
>   ccan/list/list.h     |  3 +++
>   ccan/minmax/minmax.h |  2 ++
>   test/accel_test.h    |  2 ++
>   test/dsa.c           |  4 ++--
>   test/dsa_test.c      |  1 +
>   test/iaa_test.c      |  1 +
>   util/log.h           | 30 +++++++++++++++---------------
>   util/util.h          |  1 +
>   util/wrapper.c       | 13 ++++++-------
>   11 files changed, 62 insertions(+), 57 deletions(-)
> 
> diff --git a/accfg/idxd.h b/accfg/idxd.h
> index 19d5435..f8b0c9a 100644
> --- a/accfg/idxd.h
> +++ b/accfg/idxd.h
> @@ -9,28 +9,26 @@
>   #include <linux/ioctl.h>
>   
>   /* Driver command error status */
> -enum idxd_scmd_stat {
> -	IDXD_SCMD_DEV_ENABLED = 0x80000010,
> -	IDXD_SCMD_DEV_NOT_ENABLED = 0x80000020,
> -	IDXD_SCMD_WQ_ENABLED = 0x80000021,
> -	IDXD_SCMD_DEV_DMA_ERR = 0x80020000,
> -	IDXD_SCMD_WQ_NO_GRP = 0x80030000,
> -	IDXD_SCMD_WQ_NO_NAME = 0x80040000,
> -	IDXD_SCMD_WQ_NO_SVM = 0x80050000,
> -	IDXD_SCMD_WQ_NO_THRESH = 0x80060000,
> -	IDXD_SCMD_WQ_PORTAL_ERR = 0x80070000,
> -	IDXD_SCMD_WQ_RES_ALLOC_ERR = 0x80080000,
> -	IDXD_SCMD_PERCPU_ERR = 0x80090000,
> -	IDXD_SCMD_DMA_CHAN_ERR = 0x800a0000,
> -	IDXD_SCMD_CDEV_ERR = 0x800b0000,
> -	IDXD_SCMD_WQ_NO_SWQ_SUPPORT = 0x800c0000,
> -	IDXD_SCMD_WQ_NONE_CONFIGURED = 0x800d0000,
> -	IDXD_SCMD_WQ_NO_SIZE = 0x800e0000,
> -	IDXD_SCMD_WQ_NO_PRIV = 0x800f0000,
> -	IDXD_SCMD_WQ_IRQ_ERR = 0x80100000,
> -	IDXD_SCMD_WQ_NO_DRV_NAME = 0x80200000,
> -	IDXD_SCMD_DEV_EVL_ERR = 0x80300000,
> -};
> +#define IDXD_SCMD_DEV_ENABLED		0x80000010
> +#define IDXD_SCMD_DEV_NOT_ENABLED	0x80000020
> +#define IDXD_SCMD_WQ_ENABLED		0x80000021
> +#define IDXD_SCMD_DEV_DMA_ERR		0x80020000
> +#define IDXD_SCMD_WQ_NO_GRP		0x80030000
> +#define IDXD_SCMD_WQ_NO_NAME		0x80040000
> +#define IDXD_SCMD_WQ_NO_SVM		0x80050000
> +#define IDXD_SCMD_WQ_NO_THRESH		0x80060000
> +#define IDXD_SCMD_WQ_PORTAL_ERR		0x80070000
> +#define IDXD_SCMD_WQ_RES_ALLOC_ERR	0x80080000
> +#define IDXD_SCMD_PERCPU_ERR		0x80090000
> +#define IDXD_SCMD_DMA_CHAN_ERR		0x800a0000
> +#define IDXD_SCMD_CDEV_ERR		0x800b0000
> +#define IDXD_SCMD_WQ_NO_SWQ_SUPPORT	0x800c0000
> +#define IDXD_SCMD_WQ_NONE_CONFIGURED	0x800d0000
> +#define IDXD_SCMD_WQ_NO_SIZE		0x800e0000
> +#define IDXD_SCMD_WQ_NO_PRIV		0x800f0000
> +#define IDXD_SCMD_WQ_IRQ_ERR		0x80100000
> +#define IDXD_SCMD_WQ_NO_DRV_NAME	0x80200000
> +#define IDXD_SCMD_DEV_EVL_ERR		0x80300000
>   
>   #define IDXD_SCMD_SOFTERR_MASK	0x80000000
>   #define IDXD_SCMD_SOFTERR_SHIFT	16
> diff --git a/accfg/lib/libaccfg.c b/accfg/lib/libaccfg.c
> index 292ce45..82e524f 100644
> --- a/accfg/lib/libaccfg.c
> +++ b/accfg/lib/libaccfg.c
> @@ -66,10 +66,8 @@ static unsigned int accfg_device_compl_size[] = {
>   	[ACCFG_DEVICE_IAX] = 64,
>   };
>   
> -enum {
> -	ACCFG_CMD_STATUS_MAX = 0x45,
> -	ACCFG_CMD_STATUS_ERROR = 0x80010000,
> -};
> +#define ACCFG_CMD_STATUS_MAX	0x45
> +#define ACCFG_CMD_STATUS_ERROR	0x80010000
>   
>   #define SCMD_STAT(x) (((x) & ~IDXD_SCMD_SOFTERR_MASK) >> \
>   		IDXD_SCMD_SOFTERR_SHIFT)
> @@ -1764,13 +1762,13 @@ ACCFG_EXPORT int accfg_group_get_##field( \
>   	return group->field; \
>   }
>   
> -accfg_group_get_field(group, read_buffers_reserved);
> -accfg_group_get_field(group, read_buffers_allowed);
> -accfg_group_get_field(group, use_read_buffer_limit);
> -accfg_group_get_field(group, traffic_class_a);
> -accfg_group_get_field(group, traffic_class_b);
> -accfg_group_get_field(group, desc_progress_limit);
> -accfg_group_get_field(group, batch_progress_limit);
> +accfg_group_get_field(group, read_buffers_reserved)
> +accfg_group_get_field(group, read_buffers_allowed)
> +accfg_group_get_field(group, use_read_buffer_limit)
> +accfg_group_get_field(group, traffic_class_a)
> +accfg_group_get_field(group, traffic_class_b)
> +accfg_group_get_field(group, desc_progress_limit)
> +accfg_group_get_field(group, batch_progress_limit)
>   
>   static void wqs_init(struct accfg_device *device)
>   {
> diff --git a/ccan/list/list.h b/ccan/list/list.h
> index 68e3dea..6080e0b 100644
> --- a/ccan/list/list.h
> +++ b/ccan/list/list.h
> @@ -8,6 +8,9 @@
>   #include <ccan/container_of/container_of.h>
>   #include <ccan/check_type/check_type.h>
>   
> +#pragma GCC diagnostic ignored "-Wpedantic"
> +#pragma GCC diagnostic ignored "-Wformat"
> +
>   /**
>    * struct list_node - an entry in a doubly-linked list
>    * @next: next entry (self if empty)
> diff --git a/ccan/minmax/minmax.h b/ccan/minmax/minmax.h
> index 54f246c..c57b736 100644
> --- a/ccan/minmax/minmax.h
> +++ b/ccan/minmax/minmax.h
> @@ -4,6 +4,8 @@
>   
>   #include "config.h"
>   
> +#pragma GCC diagnostic ignored "-Wpedantic"
> +
>   #include <ccan/build_assert/build_assert.h>
>   
>   #if !HAVE_STATEMENT_EXPR || !HAVE_TYPEOF
> diff --git a/test/accel_test.h b/test/accel_test.h
> index 3b41a78..6aabfde 100644
> --- a/test/accel_test.h
> +++ b/test/accel_test.h
> @@ -6,6 +6,8 @@
>   #include <accfg/idxd.h>
>   #include "accfg_test.h"
>   
> +#pragma GCC diagnostic ignored "-Wpedantic"
> +
>   #define SHARED 1
>   #define DEDICATED 0
>   
> diff --git a/test/dsa.c b/test/dsa.c
> index 54000a8..5638147 100644
> --- a/test/dsa.c
> +++ b/test/dsa.c
> @@ -223,7 +223,7 @@ int init_crcgen(struct task *tsk, int tflags, int opcode, unsigned long xfer_siz
>   	memset_pattern(tsk->src1, tsk->pattern, xfer_size);
>   	tsk->crc_seed = 0x12345678;
>   	if (tsk->test_flags & (unsigned int)(READ_CRC_SEED)) {
> -		tsk->crc_seed_addr = aligned_alloc(ADDR_ALIGNMENT, sizeof(tsk->crc_seed));
> +		tsk->crc_seed_addr = aligned_alloc(ADDR_ALIGNMENT, sizeof(*tsk->crc_seed_addr));
>   		*tsk->crc_seed_addr = tsk->crc_seed;
>   		tsk->crc_seed = 0x0;
>   	}
> @@ -253,7 +253,7 @@ int init_copy_crc(struct task *tsk, int tflags, int opcode, unsigned long xfer_s
>   
>   	tsk->crc_seed = 0x12345678;
>   	if (tsk->test_flags & (unsigned int)(READ_CRC_SEED)) {
> -		tsk->crc_seed_addr = aligned_alloc(ADDR_ALIGNMENT, sizeof(tsk->crc_seed));
> +		tsk->crc_seed_addr = aligned_alloc(ADDR_ALIGNMENT, sizeof(*tsk->crc_seed_addr));
>   		*tsk->crc_seed_addr = tsk->crc_seed;
>   		tsk->crc_seed = 0x0;
>   	}
> diff --git a/test/dsa_test.c b/test/dsa_test.c
> index a4aad29..9f9d99b 100644
> --- a/test/dsa_test.c
> +++ b/test/dsa_test.c
> @@ -13,6 +13,7 @@
>   #include "dsa.h"
>   
>   #define DSA_TEST_SIZE 20000
> +#pragma GCC diagnostic ignored "-Wformat"
>   
>   static void usage(void)
>   {
> diff --git a/test/iaa_test.c b/test/iaa_test.c
> index a1f529d..de9b860 100644
> --- a/test/iaa_test.c
> +++ b/test/iaa_test.c
> @@ -9,6 +9,7 @@
>   #include "iaa.h"
>   
>   #define IAA_TEST_SIZE 20000
> +#pragma GCC diagnostic ignored "-Wformat"
>   
>   static void usage(void)
>   {
> diff --git a/util/log.h b/util/log.h
> index dc39e9b..2734c94 100644
> --- a/util/log.h
> +++ b/util/log.h
> @@ -25,32 +25,32 @@ void log_init(struct log_ctx *ctx, const char *owner, const char *log_env);
>   static inline void __attribute__((always_inline, format(printf, 2, 3)))
>   	log_null(struct log_ctx *ctx, const char *format, ...) {}
>   
> -#define log_cond(ctx, prio, arg...) \
> +#define log_cond(ctx, prio, ...) \
>   do { \
>   	if ((ctx)->log_priority >= prio) \
> -		do_log(ctx, prio, __FILE__, __LINE__, __FUNCTION__, ## arg); \
> +		do_log(ctx, prio, __FILE__, __LINE__, __func__, __VA_ARGS__); \
>   } while (0)
>   
>   #ifdef ENABLE_LOGGING
>   #  ifdef ENABLE_DEBUG
> -#    define log_dbg(ctx, arg...) log_cond(ctx, LOG_DEBUG, ## arg)
> +#    define log_dbg(ctx, ...) log_cond(ctx, LOG_DEBUG, __VA_ARGS__)
>   #  else
> -#    define log_dbg(ctx, arg...) log_null(ctx, ## arg)
> +#    define log_dbg(ctx, ...) log_null(ctx, __VA_ARGS__)
>   #  endif
> -#  define log_info(ctx, arg...) log_cond(ctx, LOG_INFO, ## arg)
> -#  define log_err(ctx, arg...) log_cond(ctx, LOG_ERR, ## arg)
> -#  define log_notice(ctx, arg...) log_cond(ctx, LOG_NOTICE, ## arg)
> +#  define log_info(ctx, ...) log_cond(ctx, LOG_INFO, __VA_ARGS__)
> +#  define log_err(ctx, ...) log_cond(ctx, LOG_ERR,  __VA_ARGS__)
> +#  define log_notice(ctx, ...) log_cond(ctx, LOG_NOTICE, __VA_ARGS__)
>   #else
> -#  define log_dbg(ctx, arg...) log_null(ctx, ## arg)
> -#  define log_info(ctx, arg...) log_null(ctx, ## arg)
> -#  define log_err(ctx, arg...) log_null(ctx, ## arg)
> -#  define log_notice(ctx, arg...) log_null(ctx, ## arg)
> +#  define log_dbg(ctx, ...) log_null(ctx, __VA_ARGS__)
> +#  define log_info(ctx, ...) log_null(ctx, __VA_ARGS__)
> +#  define log_err(ctx, ...) log_null(ctx, __VA_ARGS__)
> +#  define log_notice(ctx, ...) log_null(ctx, __VA_ARGS__)
>   #endif
>   
> -#define dbg(x, arg...) log_dbg(&(x)->ctx, ## arg)
> -#define info(x, arg...) log_info(&(x)->ctx, ## arg)
> -#define err(x, arg...) log_err(&(x)->ctx, ## arg)
> -#define notice(x, arg...) log_notice(&(x)->ctx, ## arg)
> +#define dbg(x, ...) log_dbg(&(x)->ctx, __VA_ARGS__)
> +#define info(x, ...) log_info(&(x)->ctx, __VA_ARGS__)
> +#define err(x, ...) log_err(&(x)->ctx, __VA_ARGS__)
> +#define notice(x, ...) log_notice(&(x)->ctx, __VA_ARGS__)
>   
>   #ifndef HAVE_SECURE_GETENV
>   #  ifdef HAVE___SECURE_GETENV
> diff --git a/util/util.h b/util/util.h
> index 001707e..2573ee0 100644
> --- a/util/util.h
> +++ b/util/util.h
> @@ -21,6 +21,7 @@
>   #include <string.h>
>   
>   #pragma GCC diagnostic ignored "-Wmissing-prototypes"
> +#pragma GCC diagnostic ignored "-Wpedantic"
>   
>   #ifdef __GNUC__
>   #define NORETURN __attribute__((__noreturn__))
> diff --git a/util/wrapper.c b/util/wrapper.c
> index b0de7bc..5460c7a 100644
> --- a/util/wrapper.c
> +++ b/util/wrapper.c
> @@ -36,14 +36,13 @@ char *xstrdup(const char *str)
>   void *xrealloc(void *ptr, size_t size)
>   {
>   	void *ret = realloc(ptr, size);
> -	if (!ret && !size)
> -		ret = realloc(ptr, 1);
> +
>   	if (!ret) {
> -		ret = realloc(ptr, size);
> -		if (!ret && !size)
> -			ret = realloc(ptr, 1);
> -		if (!ret)
> -			die("Out of memory, realloc failed");
> +		/* realloc() would free ptr if size == 0 */
> +		if (size)
> +			free(ptr);
> +
> +		die("Out of memory, realloc failed");
>   	}
>   	return ret;
>   }