libmtp-discuss

[Libmtp-discuss] Fixed some cppcheck warnings

[Reverend H. <mk....@gm...>]

There are some cppcheck "warning"s cleaned. There were just format issues

[Libmtp-discuss] [PATCH 1/3] libmtp: fix a couple of format strings

[Reverend H. <mk....@gm...>]

[src/libmtp.c:3379]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.
[src/libmtp.c:3379]: (warning) %d in format string (no. 2) requires 'int' but the argument type is 'unsigned int'.
[src/libmtp.c:3379]: (warning) %d in format string (no. 3) requires 'int' but the argument type is 'unsigned int'.
[src/libmtp.c:3405]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.
---
 src/libmtp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/libmtp.c b/src/libmtp.c
index 23bb3d2..483ac18 100644
--- a/src/libmtp.c
+++ b/src/libmtp.c
@@ -3376,7 +3376,7 @@ void LIBMTP_Dump_Device_Info(LIBMTP_mtpdevice_t *device)
 	      printf(" UINT32 data type");
 	      switch (opd.FormFlag) {
 	      case PTP_OPFF_Range:
-		printf(" range: MIN %d, MAX %d, STEP %d",
+		printf(" range: MIN %u, MAX %u, STEP %u",
 		       opd.FORM.Range.MinimumValue.u32,
 		       opd.FORM.Range.MaximumValue.u32,
 		       opd.FORM.Range.StepSize.u32);
@@ -3402,7 +3402,7 @@ void LIBMTP_Dump_Device_Info(LIBMTP_mtpdevice_t *device)
 		} else {
 		  printf(" enumeration: ");
 		  for(k=0;k<opd.FORM.Enum.NumberOfValues;k++) {
-		    printf("%d, ", opd.FORM.Enum.SupportedValue[k].u32);
+		    printf("%u, ", opd.FORM.Enum.SupportedValue[k].u32);
 		  }
 		}
 		break;
-- 
2.19.2

[Libmtp-discuss] [PATCH 2/3] ptp: fix a bunch of format warnings

[Reverend H. <mk....@gm...>]

[src/ptp.c:568]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
[src/ptp.c:588]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
[src/ptp.c:608]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
[src/ptp.c:620]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
[src/ptp.c:625]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
[src/ptp.c:664]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
[src/ptp.c:675]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
[src/ptp.c:1108]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
[src/ptp.c:1114]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
[src/ptp.c:1962]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
---
 src/ptp.c | 72 ++++++++++++++++++++++++-------------------------------
 1 file changed, 31 insertions(+), 41 deletions(-)

diff --git a/src/ptp.c b/src/ptp.c
index d66c8f2..b2bd8bc 100644
--- a/src/ptp.c
+++ b/src/ptp.c
@@ -552,83 +552,71 @@ parse_9301_cmd_tree (PTPParams *params, xmlNodePtr node, PTPDeviceInfo *di)
 static int
 parse_9301_value (PTPParams *params, const char *str, uint16_t type, PTPPropertyValue *propval)
 {
+	unsigned x;
 	switch (type) {
-	case 6: { /*UINT32*/
-		unsigned int x;
+	case 6: /*UINT32*/
 		if (!sscanf(str,"%08x", &x)) {
 			ptp_debug( params, "could not parse uint32 %s", str);
 			return PTP_RC_GeneralError;
 		}
-		ptp_debug( params, "\t%d", x);
+		ptp_debug( params, "\t%u", x);
 		propval->u32 = x;
 		break;
-	}
-	case 5: { /*INT32*/
-		int x;
+	case 5: /*INT32*/
 		if (!sscanf(str,"%08x", &x)) {
 			ptp_debug( params, "could not parse int32 %s", str);
 			return PTP_RC_GeneralError;
 		}
-		ptp_debug( params, "\t%d", x);
-		propval->i32 = x;
+		ptp_debug( params, "\t%u", x);
+		propval->i32 = (int32_t)x;
 		break;
-	}
-	case 4: { /*UINT16*/
-		unsigned int x;
+	case 4: /*UINT16*/
 		if (!sscanf(str,"%04x", &x)) {
 			ptp_debug( params, "could not parse uint16 %s", str);
 			return PTP_RC_GeneralError;
 		}
-		ptp_debug( params, "\t%d", x);
+		ptp_debug( params, "\t%u", x);
 		propval->u16 = x;
 		break;
-	}
-	case 3: { /*INT16*/
-		int x;
+	case 3: /*INT16*/
 		if (!sscanf(str,"%04x", &x)) {
 			ptp_debug( params, "could not parse int16 %s", str);
 			return PTP_RC_GeneralError;
 		}
-		ptp_debug( params, "\t%d", x);
-		propval->i16 = x;
+		ptp_debug( params, "\t%u", x);
+		propval->i16 = (int16_t)x;
 		break;
-	}
-	case 2: { /*UINT8*/
-		unsigned int x;
+	case 2: /*UINT8*/
 		if (!sscanf(str,"%02x", &x)) {
 			ptp_debug( params, "could not parse uint8 %s", str);
 			return PTP_RC_GeneralError;
 		}
-		ptp_debug( params, "\t%d", x);
+		ptp_debug( params, "\t%u", x);
 		propval->u8 = x;
 		break;
-	}
-	case 1: { /*INT8*/
-		int x;
+	case 1: /*INT8*/
+		unsigned int x;
 		if (!sscanf(str,"%02x", &x)) {
 			ptp_debug( params, "could not parse int8 %s", str);
 			return PTP_RC_GeneralError;
 		} 
-		ptp_debug( params, "\t%d", x);
+		ptp_debug( params, "\t%u", x);
 		propval->i8 = x;
 		break;
-	}
-	case 65535: { /* string */
-		int len;
-
+	case 65535: /* string */
 		/* ascii ptp string, 1 byte length, little endian 16 bit chars */
-		if (sscanf(str,"%02x", &len)) {
-			int i;
-			char *xstr = malloc(len+1);
-			for (i=0;i<len;i++) {
-				int xc;
+		if (sscanf(str,"%02x", &x)) {
+			unsigned i;
+			char *xstr = malloc(x+1);
+			for (i=0;i<x;i++) {
+				unsigned xc;
 				if (sscanf(str+2+i*4,"%04x", &xc)) {
 					int cx;
 
 					cx = ((xc>>8) & 0xff) | ((xc & 0xff) << 8);
 					xstr[i] = cx;
 				}
-				xstr[len] = 0;
+				xstr[x] = 0;
 			}
 			ptp_debug( params, "\t%s", xstr);
 			propval->str = xstr;
@@ -636,7 +624,6 @@ parse_9301_value (PTPParams *params, const char *str, uint16_t type, PTPProperty
 		}
 		ptp_debug( params, "string %s not parseable!", str);
 		return PTP_RC_GeneralError;
-	}
 	case 7: /*INT64*/
 	case 8: /*UINT64*/
 	case 9: /*INT128*/
@@ -651,7 +638,7 @@ parse_9301_value (PTPParams *params, const char *str, uint16_t type, PTPProperty
 static int
 parse_9301_propdesc (PTPParams *params, xmlNodePtr next, PTPDevicePropDesc *dpd)
 {
-	int type = -1;
+	unsigned type = (unsigned)-1;
 
 	if (!next)
 		return PTP_RC_GeneralError;
@@ -670,7 +657,7 @@ parse_9301_propdesc (PTPParams *params, xmlNodePtr next, PTPDevicePropDesc *dpd)
 			continue;
 		}
 		if (!strcmp((char*)next->name,"attribute")) {	/* propdesc.GetSet */
-			int attr;
+			unsigned attr;
 
 			if (!sscanf((char*)xmlNodeGetContent (next), "%02x", &attr)) {
 				ptp_debug( params, "\tattr %s not parseable",xmlNodeGetContent (next));
@@ -1068,7 +1055,7 @@ ptp_olympus_parse_output_xml(PTPParams* params, char*data, int len, xmlNodePtr *
 {
         xmlDocPtr       docin;
         xmlNodePtr      docroot, output, next;
-	int 		result, xcode;
+	int 		result;
 
 	*code = NULL;
 
@@ -1101,12 +1088,15 @@ ptp_olympus_parse_output_xml(PTPParams* params, char*data, int len, xmlNodePtr *
 	result = PTP_RC_GeneralError;
 
 	while (next) {
+		unsigned xcode;
 		if (!strcmp((char*)next->name,"result")) {
 			xmlChar	 *xchar;
+			unsigned ptp_res;
 
 			xchar = xmlNodeGetContent (next);
-			if (!sscanf((char*)xchar,"%04x",&result))
+			if (!sscanf((char*)xchar,"%04x",&ptp_res))
 				ptp_debug (params, "failed scanning result from %s", xchar);
+			result = ptp_res;
 			ptp_debug (params,  "ptp result is 0x%04x", result);
 			next = xmlNextElementSibling (next);
 			continue;
@@ -1952,7 +1942,7 @@ ptp_getdevicepropdesc (PTPParams* params, uint16_t propcode,
 
 		ret = ptp_olympus_parse_output_xml (params,(char*)data,size,&code);
 		if (ret == PTP_RC_OK) {
-			int x;
+			unsigned x;
 
 			if (	(xmlChildElementCount(code) == 1) &&
 					(!strcmp((char*)code->name,"c1014"))
-- 
2.19.2

[Libmtp-discuss] [PATCH 3/3] ptp-pack: fix some format issues

[Reverend H. <mk....@gm...>]

[src/ptp-pack.c:2699]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.
[src/ptp-pack.c:2700]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.
[src/ptp-pack.c:2701]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.
[src/ptp-pack.c:2702]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.
[src/ptp-pack.c:2703]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.
[src/ptp-pack.c:2704]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.
[src/ptp-pack.c:2705]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.
[src/ptp-pack.c:2706]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.
[src/ptp-pack.c:2707]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.
[src/ptp-pack.c:2708]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.
[src/ptp-pack.c:2709]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.
[src/ptp-pack.c:2710]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.
[src/ptp-pack.c:2711]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.
[src/ptp-pack.c:2712]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.
[src/ptp-pack.c:2713]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.
[src/ptp-pack.c:2714]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.
[src/ptp-pack.c:2715]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.
---
 src/ptp-pack.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/ptp-pack.c b/src/ptp-pack.c
index 5319525..02ef2cf 100644
--- a/src/ptp-pack.c
+++ b/src/ptp-pack.c
@@ -1783,7 +1783,7 @@ ptp_unpack_EOS_FocusInfoEx (PTPParams* params, unsigned char** data, uint32_t da
 		return NULL;
 	p = str;
 
-	p += sprintf(p,"eosversion=%d,size=%dx%d,size2=%dx%d,points={", version, sizeX, sizeY, size2X, size2Y);
+	p += sprintf(p,"eosversion=%u,size=%ux%u,size2=%ux%u,points={", version, sizeX, sizeY, size2X, size2Y);
 	for (i=0;i<focus_points_in_use;i++) {
 		int16_t x = dtoh16a((*data) + focus_points_in_struct*4 + 20 + 2*i);
 		int16_t y = dtoh16a((*data) + focus_points_in_struct*6 + 20 + 2*i);
@@ -1798,7 +1798,7 @@ ptp_unpack_EOS_FocusInfoEx (PTPParams* params, unsigned char** data, uint32_t da
 	p += sprintf(p,"},select={");
 	for (i=0;i<focus_points_in_use;i++) {
 		if ((1<<(i%8)) & ((*data)[focus_points_in_struct*8+20+i/8]))
-			p+=sprintf(p,"%d,", i);
+			p+=sprintf(p,"%u,", i);
 	}
 
 	p += sprintf(p,"},unknown={");
@@ -2102,12 +2102,12 @@ ptp_unpack_CANON_changes (PTPParams *params, unsigned char* data, int datasize,
 				switch (dpd->DataType) {
 #define XX( TYPE, CONV )\
 					if (sizeof(dpd->FORM.Enum.SupportedValue[j].TYPE)*propxcnt + PTP_ece_Prop_Desc_Data > size) {	\
-						ptp_debug (params, "size %d does not match needed %d", sizeof(dpd->FORM.Enum.SupportedValue[j].TYPE)*propxcnt + PTP_ece_Prop_Desc_Data, size);	\
+						ptp_debug (params, "size %lu does not match needed %u", sizeof(dpd->FORM.Enum.SupportedValue[j].TYPE)*propxcnt + PTP_ece_Prop_Desc_Data, size);	\
 						break;							\
 					}								\
 					for (j=0;j<propxcnt;j++) { 					\
 						dpd->FORM.Enum.SupportedValue[j].TYPE = CONV(xdata); 	\
-						ptp_debug (params, "event %d: suppval[%d] of %x is 0x%x.", i, j, proptype, CONV(xdata)); \
+						ptp_debug (params, "event %u: suppval[%u] of %x is 0x%x.", i, j, proptype, CONV(xdata)); \
 						xdata += 4; /* might only be for propxtype 3 */ \
 					} \
 					break;
@@ -2683,7 +2683,7 @@ ptp_unpack_CANON_changes (PTPParams *params, unsigned char* data, int datasize,
 		case PTP_EC_CANON_EOS_BulbExposureTime:
 			ce[i].type = PTP_CANON_EOS_CHANGES_TYPE_UNKNOWN;
 			ce[i].u.info = malloc(strlen("BulbExposureTime 123456789012345678"));
-			sprintf (ce[i].u.info, "BulbExposureTime %d",  dtoh32a(curdata+8));
+			sprintf (ce[i].u.info, "BulbExposureTime %u",  dtoh32a(curdata+8));
 			break;
 		case PTP_EC_CANON_EOS_ObjectRemoved:
 			ce[i].type = PTP_CANON_EOS_CHANGES_TYPE_OBJECTREMOVED;
@@ -2692,9 +2692,9 @@ ptp_unpack_CANON_changes (PTPParams *params, unsigned char* data, int datasize,
 		default:
 			switch (type) {
 #define XX(x)		case PTP_EC_CANON_EOS_##x: 								\
-				ptp_debug (params, "event %d: unhandled EOS event "#x" (size %d)", i, size); 	\
+				ptp_debug (params, "event %u: unhandled EOS event "#x" (size %u)", i, size); 	\
 				ce[i].u.info = malloc(strlen("unhandled EOS event "#x" (size 123456789)"));	\
-				sprintf (ce[i].u.info, "unhandled EOS event "#x" (size %d)",  size);		\
+				sprintf (ce[i].u.info, "unhandled EOS event "#x" (size %u)",  size);		\
 				break;
 			XX(RequestGetEvent)
 			XX(RequestGetObjectInfoEx)
-- 
2.19.2

Re: [Libmtp-discuss] [PATCH 2/3] ptp: fix a bunch of format warnings

[Marcus M. <ma...@je...>]

Hi,

While I applied all others patches, this patch changes other things.

Casting between unsigned and signed integers is worse than using incorrect format strings.

Perhaps you can use two variables for x, one signed and one unsigned.

Ciao, Marcus

On Thu, Nov 29, 2018 at 04:59:30PM +0300, Reverend Homer wrote:
> [src/ptp.c:568]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
> [src/ptp.c:588]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
> [src/ptp.c:608]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
> [src/ptp.c:620]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
> [src/ptp.c:625]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
> [src/ptp.c:664]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
> [src/ptp.c:675]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
> [src/ptp.c:1108]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
> [src/ptp.c:1114]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
> [src/ptp.c:1962]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
> ---
>  src/ptp.c | 72 ++++++++++++++++++++++++-------------------------------
>  1 file changed, 31 insertions(+), 41 deletions(-)
> 
> diff --git a/src/ptp.c b/src/ptp.c
> index d66c8f2..b2bd8bc 100644
> --- a/src/ptp.c
> +++ b/src/ptp.c
> @@ -552,83 +552,71 @@ parse_9301_cmd_tree (PTPParams *params, xmlNodePtr node, PTPDeviceInfo *di)
>  static int
>  parse_9301_value (PTPParams *params, const char *str, uint16_t type, PTPPropertyValue *propval)
>  {
> +	unsigned x;
>  	switch (type) {
> -	case 6: { /*UINT32*/
> -		unsigned int x;
> +	case 6: /*UINT32*/
>  		if (!sscanf(str,"%08x", &x)) {
>  			ptp_debug( params, "could not parse uint32 %s", str);
>  			return PTP_RC_GeneralError;
>  		}
> -		ptp_debug( params, "\t%d", x);
> +		ptp_debug( params, "\t%u", x);
>  		propval->u32 = x;
>  		break;
> -	}
> -	case 5: { /*INT32*/
> -		int x;
> +	case 5: /*INT32*/
>  		if (!sscanf(str,"%08x", &x)) {
>  			ptp_debug( params, "could not parse int32 %s", str);
>  			return PTP_RC_GeneralError;
>  		}
> -		ptp_debug( params, "\t%d", x);
> -		propval->i32 = x;
> +		ptp_debug( params, "\t%u", x);
> +		propval->i32 = (int32_t)x;
>  		break;
> -	}
> -	case 4: { /*UINT16*/
> -		unsigned int x;
> +	case 4: /*UINT16*/
>  		if (!sscanf(str,"%04x", &x)) {
>  			ptp_debug( params, "could not parse uint16 %s", str);
>  			return PTP_RC_GeneralError;
>  		}
> -		ptp_debug( params, "\t%d", x);
> +		ptp_debug( params, "\t%u", x);
>  		propval->u16 = x;
>  		break;
> -	}
> -	case 3: { /*INT16*/
> -		int x;
> +	case 3: /*INT16*/
>  		if (!sscanf(str,"%04x", &x)) {
>  			ptp_debug( params, "could not parse int16 %s", str);
>  			return PTP_RC_GeneralError;
>  		}
> -		ptp_debug( params, "\t%d", x);
> -		propval->i16 = x;
> +		ptp_debug( params, "\t%u", x);
> +		propval->i16 = (int16_t)x;
>  		break;
> -	}
> -	case 2: { /*UINT8*/
> -		unsigned int x;
> +	case 2: /*UINT8*/
>  		if (!sscanf(str,"%02x", &x)) {
>  			ptp_debug( params, "could not parse uint8 %s", str);
>  			return PTP_RC_GeneralError;
>  		}
> -		ptp_debug( params, "\t%d", x);
> +		ptp_debug( params, "\t%u", x);
>  		propval->u8 = x;
>  		break;
> -	}
> -	case 1: { /*INT8*/
> -		int x;
> +	case 1: /*INT8*/
> +		unsigned int x;
>  		if (!sscanf(str,"%02x", &x)) {
>  			ptp_debug( params, "could not parse int8 %s", str);
>  			return PTP_RC_GeneralError;
>  		} 
> -		ptp_debug( params, "\t%d", x);
> +		ptp_debug( params, "\t%u", x);
>  		propval->i8 = x;
>  		break;
> -	}
> -	case 65535: { /* string */
> -		int len;
> -
> +	case 65535: /* string */
>  		/* ascii ptp string, 1 byte length, little endian 16 bit chars */
> -		if (sscanf(str,"%02x", &len)) {
> -			int i;
> -			char *xstr = malloc(len+1);
> -			for (i=0;i<len;i++) {
> -				int xc;
> +		if (sscanf(str,"%02x", &x)) {
> +			unsigned i;
> +			char *xstr = malloc(x+1);
> +			for (i=0;i<x;i++) {
> +				unsigned xc;
>  				if (sscanf(str+2+i*4,"%04x", &xc)) {
>  					int cx;
>  
>  					cx = ((xc>>8) & 0xff) | ((xc & 0xff) << 8);
>  					xstr[i] = cx;
>  				}
> -				xstr[len] = 0;
> +				xstr[x] = 0;
>  			}
>  			ptp_debug( params, "\t%s", xstr);
>  			propval->str = xstr;
> @@ -636,7 +624,6 @@ parse_9301_value (PTPParams *params, const char *str, uint16_t type, PTPProperty
>  		}
>  		ptp_debug( params, "string %s not parseable!", str);
>  		return PTP_RC_GeneralError;
> -	}
>  	case 7: /*INT64*/
>  	case 8: /*UINT64*/
>  	case 9: /*INT128*/
> @@ -651,7 +638,7 @@ parse_9301_value (PTPParams *params, const char *str, uint16_t type, PTPProperty
>  static int
>  parse_9301_propdesc (PTPParams *params, xmlNodePtr next, PTPDevicePropDesc *dpd)
>  {
> -	int type = -1;
> +	unsigned type = (unsigned)-1;
>  
>  	if (!next)
>  		return PTP_RC_GeneralError;
> @@ -670,7 +657,7 @@ parse_9301_propdesc (PTPParams *params, xmlNodePtr next, PTPDevicePropDesc *dpd)
>  			continue;
>  		}
>  		if (!strcmp((char*)next->name,"attribute")) {	/* propdesc.GetSet */
> -			int attr;
> +			unsigned attr;
>  
>  			if (!sscanf((char*)xmlNodeGetContent (next), "%02x", &attr)) {
>  				ptp_debug( params, "\tattr %s not parseable",xmlNodeGetContent (next));
> @@ -1068,7 +1055,7 @@ ptp_olympus_parse_output_xml(PTPParams* params, char*data, int len, xmlNodePtr *
>  {
>          xmlDocPtr       docin;
>          xmlNodePtr      docroot, output, next;
> -	int 		result, xcode;
> +	int 		result;
>  
>  	*code = NULL;
>  
> @@ -1101,12 +1088,15 @@ ptp_olympus_parse_output_xml(PTPParams* params, char*data, int len, xmlNodePtr *
>  	result = PTP_RC_GeneralError;
>  
>  	while (next) {
> +		unsigned xcode;
>  		if (!strcmp((char*)next->name,"result")) {
>  			xmlChar	 *xchar;
> +			unsigned ptp_res;
>  
>  			xchar = xmlNodeGetContent (next);
> -			if (!sscanf((char*)xchar,"%04x",&result))
> +			if (!sscanf((char*)xchar,"%04x",&ptp_res))
>  				ptp_debug (params, "failed scanning result from %s", xchar);
> +			result = ptp_res;
>  			ptp_debug (params,  "ptp result is 0x%04x", result);
>  			next = xmlNextElementSibling (next);
>  			continue;
> @@ -1952,7 +1942,7 @@ ptp_getdevicepropdesc (PTPParams* params, uint16_t propcode,
>  
>  		ret = ptp_olympus_parse_output_xml (params,(char*)data,size,&code);
>  		if (ret == PTP_RC_OK) {
> -			int x;
> +			unsigned x;
>  
>  			if (	(xmlChildElementCount(code) == 1) &&
>  					(!strcmp((char*)code->name,"c1014"))
> -- 
> 2.19.2
> 
> 
> 
> _______________________________________________
> Libmtp-discuss mailing list
> Lib...@li...
> https://lists.sourceforge.net/lists/listinfo/libmtp-discuss

Re: [Libmtp-discuss] [PATCH 2/3] ptp: fix a bunch of format warnings

[Reverend H. <mk....@gm...>]

Hi Marcus,

On 05/12/2018 10:10, Marcus Meissner wrote:
> Hi,
> 
> While I applied all others patches, this patch changes other things.
> 
> Casting between unsigned and signed integers is worse than using incorrect format strings.

Actually, I think what we have now is even worse:
https://stackoverflow.com/questions/14802970/sscanf-with-hexadecimal-negative-value

Long story short: passing signed integer's address to scanf with hex parsing 
could give you unexpected results.

Regards,
R.H.

> 
> Perhaps you can use two variables for x, one signed and one unsigned.
> 
> Ciao, Marcus
> 
> On Thu, Nov 29, 2018 at 04:59:30PM +0300, Reverend Homer wrote:
>> [src/ptp.c:568]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
>> [src/ptp.c:588]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
>> [src/ptp.c:608]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
>> [src/ptp.c:620]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
>> [src/ptp.c:625]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
>> [src/ptp.c:664]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
>> [src/ptp.c:675]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
>> [src/ptp.c:1108]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
>> [src/ptp.c:1114]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
>> [src/ptp.c:1962]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
>> ---
>>   src/ptp.c | 72 ++++++++++++++++++++++++-------------------------------
>>   1 file changed, 31 insertions(+), 41 deletions(-)
>>
>> diff --git a/src/ptp.c b/src/ptp.c
>> index d66c8f2..b2bd8bc 100644
>> --- a/src/ptp.c
>> +++ b/src/ptp.c
>> @@ -552,83 +552,71 @@ parse_9301_cmd_tree (PTPParams *params, xmlNodePtr node, PTPDeviceInfo *di)
>>   static int
>>   parse_9301_value (PTPParams *params, const char *str, uint16_t type, PTPPropertyValue *propval)
>>   {
>> +	unsigned x;
>>   	switch (type) {
>> -	case 6: { /*UINT32*/
>> -		unsigned int x;
>> +	case 6: /*UINT32*/
>>   		if (!sscanf(str,"%08x", &x)) {
>>   			ptp_debug( params, "could not parse uint32 %s", str);
>>   			return PTP_RC_GeneralError;
>>   		}
>> -		ptp_debug( params, "\t%d", x);
>> +		ptp_debug( params, "\t%u", x);
>>   		propval->u32 = x;
>>   		break;
>> -	}
>> -	case 5: { /*INT32*/
>> -		int x;
>> +	case 5: /*INT32*/
>>   		if (!sscanf(str,"%08x", &x)) {
>>   			ptp_debug( params, "could not parse int32 %s", str);
>>   			return PTP_RC_GeneralError;
>>   		}
>> -		ptp_debug( params, "\t%d", x);
>> -		propval->i32 = x;
>> +		ptp_debug( params, "\t%u", x);
>> +		propval->i32 = (int32_t)x;
>>   		break;
>> -	}
>> -	case 4: { /*UINT16*/
>> -		unsigned int x;
>> +	case 4: /*UINT16*/
>>   		if (!sscanf(str,"%04x", &x)) {
>>   			ptp_debug( params, "could not parse uint16 %s", str);
>>   			return PTP_RC_GeneralError;
>>   		}
>> -		ptp_debug( params, "\t%d", x);
>> +		ptp_debug( params, "\t%u", x);
>>   		propval->u16 = x;
>>   		break;
>> -	}
>> -	case 3: { /*INT16*/
>> -		int x;
>> +	case 3: /*INT16*/
>>   		if (!sscanf(str,"%04x", &x)) {
>>   			ptp_debug( params, "could not parse int16 %s", str);
>>   			return PTP_RC_GeneralError;
>>   		}
>> -		ptp_debug( params, "\t%d", x);
>> -		propval->i16 = x;
>> +		ptp_debug( params, "\t%u", x);
>> +		propval->i16 = (int16_t)x;
>>   		break;
>> -	}
>> -	case 2: { /*UINT8*/
>> -		unsigned int x;
>> +	case 2: /*UINT8*/
>>   		if (!sscanf(str,"%02x", &x)) {
>>   			ptp_debug( params, "could not parse uint8 %s", str);
>>   			return PTP_RC_GeneralError;
>>   		}
>> -		ptp_debug( params, "\t%d", x);
>> +		ptp_debug( params, "\t%u", x);
>>   		propval->u8 = x;
>>   		break;
>> -	}
>> -	case 1: { /*INT8*/
>> -		int x;
>> +	case 1: /*INT8*/
>> +		unsigned int x;
>>   		if (!sscanf(str,"%02x", &x)) {
>>   			ptp_debug( params, "could not parse int8 %s", str);
>>   			return PTP_RC_GeneralError;
>>   		}
>> -		ptp_debug( params, "\t%d", x);
>> +		ptp_debug( params, "\t%u", x);
>>   		propval->i8 = x;
>>   		break;
>> -	}
>> -	case 65535: { /* string */
>> -		int len;
>> -
>> +	case 65535: /* string */
>>   		/* ascii ptp string, 1 byte length, little endian 16 bit chars */
>> -		if (sscanf(str,"%02x", &len)) {
>> -			int i;
>> -			char *xstr = malloc(len+1);
>> -			for (i=0;i<len;i++) {
>> -				int xc;
>> +		if (sscanf(str,"%02x", &x)) {
>> +			unsigned i;
>> +			char *xstr = malloc(x+1);
>> +			for (i=0;i<x;i++) {
>> +				unsigned xc;
>>   				if (sscanf(str+2+i*4,"%04x", &xc)) {
>>   					int cx;
>>   
>>   					cx = ((xc>>8) & 0xff) | ((xc & 0xff) << 8);
>>   					xstr[i] = cx;
>>   				}
>> -				xstr[len] = 0;
>> +				xstr[x] = 0;
>>   			}
>>   			ptp_debug( params, "\t%s", xstr);
>>   			propval->str = xstr;
>> @@ -636,7 +624,6 @@ parse_9301_value (PTPParams *params, const char *str, uint16_t type, PTPProperty
>>   		}
>>   		ptp_debug( params, "string %s not parseable!", str);
>>   		return PTP_RC_GeneralError;
>> -	}
>>   	case 7: /*INT64*/
>>   	case 8: /*UINT64*/
>>   	case 9: /*INT128*/
>> @@ -651,7 +638,7 @@ parse_9301_value (PTPParams *params, const char *str, uint16_t type, PTPProperty
>>   static int
>>   parse_9301_propdesc (PTPParams *params, xmlNodePtr next, PTPDevicePropDesc *dpd)
>>   {
>> -	int type = -1;
>> +	unsigned type = (unsigned)-1;
>>   
>>   	if (!next)
>>   		return PTP_RC_GeneralError;
>> @@ -670,7 +657,7 @@ parse_9301_propdesc (PTPParams *params, xmlNodePtr next, PTPDevicePropDesc *dpd)
>>   			continue;
>>   		}
>>   		if (!strcmp((char*)next->name,"attribute")) {	/* propdesc.GetSet */
>> -			int attr;
>> +			unsigned attr;
>>   
>>   			if (!sscanf((char*)xmlNodeGetContent (next), "%02x", &attr)) {
>>   				ptp_debug( params, "\tattr %s not parseable",xmlNodeGetContent (next));
>> @@ -1068,7 +1055,7 @@ ptp_olympus_parse_output_xml(PTPParams* params, char*data, int len, xmlNodePtr *
>>   {
>>           xmlDocPtr       docin;
>>           xmlNodePtr      docroot, output, next;
>> -	int 		result, xcode;
>> +	int 		result;
>>   
>>   	*code = NULL;
>>   
>> @@ -1101,12 +1088,15 @@ ptp_olympus_parse_output_xml(PTPParams* params, char*data, int len, xmlNodePtr *
>>   	result = PTP_RC_GeneralError;
>>   
>>   	while (next) {
>> +		unsigned xcode;
>>   		if (!strcmp((char*)next->name,"result")) {
>>   			xmlChar	 *xchar;
>> +			unsigned ptp_res;
>>   
>>   			xchar = xmlNodeGetContent (next);
>> -			if (!sscanf((char*)xchar,"%04x",&result))
>> +			if (!sscanf((char*)xchar,"%04x",&ptp_res))
>>   				ptp_debug (params, "failed scanning result from %s", xchar);
>> +			result = ptp_res;
>>   			ptp_debug (params,  "ptp result is 0x%04x", result);
>>   			next = xmlNextElementSibling (next);
>>   			continue;
>> @@ -1952,7 +1942,7 @@ ptp_getdevicepropdesc (PTPParams* params, uint16_t propcode,
>>   
>>   		ret = ptp_olympus_parse_output_xml (params,(char*)data,size,&code);
>>   		if (ret == PTP_RC_OK) {
>> -			int x;
>> +			unsigned x;
>>   
>>   			if (	(xmlChildElementCount(code) == 1) &&
>>   					(!strcmp((char*)code->name,"c1014"))
>> -- 
>> 2.19.2
>>
>>
>>
>> _______________________________________________
>> Libmtp-discuss mailing list
>> Lib...@li...
>> https://lists.sourceforge.net/lists/listinfo/libmtp-discuss
> 
> 
> _______________________________________________
> Libmtp-discuss mailing list
> Lib...@li...
> https://lists.sourceforge.net/lists/listinfo/libmtp-discuss
>