[Accel-config] Re: [PATCH] accel-config: Replace unsigned long with uint64_t

Hi Libor,

Thanks for reviewing. I will change to "SCN" in sscanfs as well as change
the places that were missed.

-Ramesh

On Wed, Mar 03, 2021 at 09:57:37AM +0100, Libor Pechacek <lpechacek(a)suse.cz> wrote:
> Thanks for CCing me on the patch, Ramesh!
> 
> On Út 02-03-21 16:48:38, ramesh.thomas(a)intel.com wrote:
> > From: Ramesh Thomas <ramesh.thomas(a)intel.com>
> >
> > Makes 64 bit data type portable across 32 bit and 62 bit targets. This
> > fixes compilation errors reported by 32 bit compilers.
> >
> > Signed-off-by: Ramesh Thomas <ramesh.thomas(a)intel.com>
> > Reported-by: Libor Pecháček <lpechacek(a)gmx.com>
> 
> Feel free to use my lpechacek(a)suse.cz address in this patch. I'm using
> the GMX one for my spare-time opensource involvement.
> 
> [...]
> 
> > @@ -678,7 +678,7 @@ static void *add_wq(void *parent, int id, const char *wq_base,
> >  	}
> >
> >  	if (sscanf(basename(wq_base_string),
> > -				"wq%ld.%ld", &device_id, &wq_id) != 2) {
> > +				"wq%" PRIu64 ".%" PRIu64, &device_id, &wq_id) != 2) {
>                                       ^^^^^^
> Just a nit. I know that the resulting format string is the same in
> this case but aren't SCN* macros intended for use with *scanf
> functions?
> 
> In addition, GCC 10 on openSUSE reported about a few more places which
> would benefit from the portable formatting strings. Otherwise,
> 
> Acked-by: Libor Pecháček <lpechacek(a)suse.cz>
> 
> Thanks!
> 
> Libor
> 
> Index: idxd-config-accel-config-v3.0.1/accfg/config_attr.c
> ===================================================================
> --- idxd-config-accel-config-v3.0.1.orig/accfg/config_attr.c
> +++ idxd-config-accel-config-v3.0.1/accfg/config_attr.c
> @@ -3,6 +3,7 @@
> 
>  #include <stdio.h>
>  #include <errno.h>
> +#include <inttypes.h>
>  #include <stdlib.h>
>  #include <time.h>
>  #include <json-c/json.h>
> @@ -184,7 +185,7 @@ static int accel_config_parse_wq_attribs
>  		|| wq_params->max_transfer_size > max_transfer_size)
>  		&& (wq_params->max_transfer_size != INT_MAX)) {
>  		fprintf(stderr,
> -			"valid max-transfer-size should be 1 to %ld\n", max_transfer_size);
> +			"valid max-transfer-size should be 1 to %" PRIu64 "\n", max_transfer_size);
>  		return -EINVAL;
>  	}
> 
> Index: idxd-config-accel-config-v3.0.1/accfg/enable.c
> ===================================================================
> --- idxd-config-accel-config-v3.0.1.orig/accfg/enable.c
> +++ idxd-config-accel-config-v3.0.1/accfg/enable.c
> @@ -3,6 +3,7 @@
> 
>  #include <stdio.h>
>  #include <errno.h>
> +#include <inttypes.h>
>  #include <stdlib.h>
>  #include <time.h>
>  #include <json-c/json.h>
> @@ -263,7 +264,7 @@ static int wq_action(int argc, const cha
>  		char dev_name[MAX_DEV_LEN], wq_name[MAX_DEV_LEN];
>  		int found = 0;
> 
> -		if (sscanf(argv[i], "%[^/]/wq%lu.%lu", dev_name, &dev_id, &wq_id) != 3) {
> +		if (sscanf(argv[i], "%[^/]/wq%" SCNu64 ".%" SCNu64, dev_name, &dev_id, &wq_id) !=
> 3) {
>  			fprintf(stderr, "'%s' is not a valid wq name\n",
>  				argv[i]);
>  			return -EINVAL;
> @@ -272,7 +273,7 @@ static int wq_action(int argc, const cha
>  		if (!accfg_device_type_validate(dev_name))
>  			return -EINVAL;
> 
> -		rc = sprintf(wq_name, "wq%lu.%lu", dev_id, wq_id);
> +		rc = sprintf(wq_name, "wq%" PRIu64 ".%" PRIu64, dev_id, wq_id);
>  		if (rc < 0)
>  			return errno;
> 
> --
> Libor Pechacek
> SUSE Labs                                Remember to have fun...