From: Marcus Glocker <marcus@nazgul.ch>
Subject: Re: sys/uvideo: replace magic constant by offest and sizeof
To: "Kirill A. Korinsky" <kirill@korins.ky>
Cc: tech@openbsd.org
Date: Sat, 6 Sep 2025 14:38:37 +0200

On Thu, Sep 04, 2025 at 09:46:27AM +0200, Kirill A. Korinsky wrote:

> tech@,
> 
> here the last step in refactoring: replace magic constant by offest and
> sizeof.
> 
> Tested on all my devices, no regression.
> 
> Ok?

ok mglocker@
 
> Index: sys/dev/usb/uvideo.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/uvideo.c,v
> diff -u -p -r1.264 uvideo.c
> --- sys/dev/usb/uvideo.c	4 Sep 2025 07:43:29 -0000	1.264
> +++ sys/dev/usb/uvideo.c	4 Sep 2025 07:44:32 -0000
> @@ -1046,45 +1046,33 @@ uvideo_vs_parse_desc_format(struct uvide
>  		/* Crossed device function boundary. */
>  		if (desc->bDescriptorType == UDESC_IFACE_ASSOC)
>  			break;
> -		if (desc->bDescriptorType != UDESC_CS_INTERFACE) {
> -			desc = usbd_desc_iter_next(&iter);
> -			continue;
> -		}
> +
> +		if (desc->bDescriptorType != UDESC_CS_INTERFACE)
> +			goto next;
> +
> +		if (desc->bLength != UVIDEO_FORMAT_LEN(desc))
> +			goto next;
>  
>  		switch (desc->bDescriptorSubtype) {
>  		case UDESCSUB_VS_COLORFORMAT:
> -			if (desc->bLength == 6) {
> -				uvideo_vs_parse_desc_colorformat(
> -				    sc, desc);
> -			}
> +			uvideo_vs_parse_desc_colorformat(sc, desc);
>  			break;
>  		case UDESCSUB_VS_FORMAT_MJPEG:
> -			if (desc->bLength == 11) {
> -				uvideo_vs_parse_desc_format_mjpeg(
> -				    sc, desc);
> -			}
> +			uvideo_vs_parse_desc_format_mjpeg(sc, desc);
>  			break;
>  		case UDESCSUB_VS_FORMAT_UNCOMPRESSED:
> -			if (desc->bLength == 27) {
> -				uvideo_vs_parse_desc_format_uncompressed(
> -				    sc, desc);
> -			}
> +			uvideo_vs_parse_desc_format_uncompressed(sc, desc);
>  			break;
>  		case UDESCSUB_VS_FORMAT_FRAME_BASED:
> -			if (desc->bLength == 28) {
> -				uvideo_vs_parse_desc_format_frame_based(
> -				    sc, desc);
> -			}
> +			uvideo_vs_parse_desc_format_frame_based(sc, desc);
>  			break;
>  		case UDESCSUB_VS_FORMAT_H264:
>  		case UDESCSUB_VS_FORMAT_H264_SIMULCAST:
> -			if (desc->bLength == 52) {
> -				uvideo_vs_parse_desc_format_h264(
> -				    sc, desc);
> -			}
> +			uvideo_vs_parse_desc_format_h264(sc, desc);
>  			break;
>  		}
>  
> +next:
>  		desc = usbd_desc_iter_next(&iter);
>  	}
>  
> @@ -2873,7 +2861,7 @@ uvideo_dump_desc_all(struct uvideo_softc
>  			case UDESCSUB_VC_SELECTOR_UNIT:
>  				printf("bDescriptorSubtype=0x%02x",
>  				    desc->bDescriptorSubtype);
> -				if (desc->bLength == 27) {
> +				if (desc->bLength == UVIDEO_FORMAT_LEN(desc)) {
>  					printf(" (UDESCSUB_VS_FORMAT_"
>  					    "UNCOMPRESSED)\n");
>  					uvideo_dump_desc_format_uncompressed(
> @@ -2901,7 +2889,7 @@ uvideo_dump_desc_all(struct uvideo_softc
>  			case UDESCSUB_VC_EXTENSION_UNIT:
>  				printf("bDescriptorSubtype=0x%02x",
>  				    desc->bDescriptorSubtype);
> -				if (desc->bLength == 11) {
> +				if (desc->bLength == UVIDEO_FORMAT_LEN(desc)) {
>  					printf(" (UDESCSUB_VS_FORMAT_MJPEG)\n");
>  					printf("|\n");
>  					uvideo_dump_desc_format_mjpeg(sc, desc);
> @@ -2925,7 +2913,7 @@ uvideo_dump_desc_all(struct uvideo_softc
>  				printf("bDescriptorSubtype=0x%02x",
>  				    desc->bDescriptorSubtype);
>  				printf(" (UDESCSUB_VS_FORMAT_FRAME_BASED)\n");
> -				if (desc->bLength == 28) {
> +				if (desc->bLength == UVIDEO_FORMAT_LEN(desc)) {
>  					printf("|\n");
>  					uvideo_dump_desc_format_frame_based(sc, desc);
>  				}
> @@ -2943,7 +2931,7 @@ uvideo_dump_desc_all(struct uvideo_softc
>  				printf("bDescriptorSubtype=0x%02x",
>  				    desc->bDescriptorSubtype);
>  				printf(" (UDESCSUB_VS_FORMAT_H264)\n");
> -				if (desc->bLength == 52) {
> +				if (desc->bLength == UVIDEO_FORMAT_LEN(desc)) {
>  					printf("|\n");
>  					uvideo_dump_desc_format_h264(sc, desc);
>  				}
> @@ -2961,7 +2949,7 @@ uvideo_dump_desc_all(struct uvideo_softc
>  				printf("bDescriptorSubtype=0x%02x",
>  				    desc->bDescriptorSubtype);
>  				printf(" (UDESCSUB_VS_FORMAT_H264_SIMULCAST)\n");
> -				if (desc->bLength == 52) {
> +				if (desc->bLength == UVIDEO_FORMAT_LEN(desc)) {
>  					printf("|\n");
>  					uvideo_dump_desc_format_h264(sc, desc);
>  				}
> Index: sys/dev/usb/uvideo.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/uvideo.h,v
> diff -u -p -r1.70 uvideo.h
> --- sys/dev/usb/uvideo.h	4 Sep 2025 07:43:29 -0000	1.70
> +++ sys/dev/usb/uvideo.h	4 Sep 2025 07:44:32 -0000
> @@ -589,6 +589,25 @@ struct usb_video_format_desc {
>  			uWord	wMaxMBperSecFourResolutionsFullScalability;
>  		} h264;
>  	} u;
> +
> +#define UVIDEO_FORMAT_LEN(fmt)							\
> +	(									\
> +	(((fmt)->bDescriptorSubtype == UDESCSUB_VS_FORMAT_H264) ||		\
> +	 ((fmt)->bDescriptorSubtype == UDESCSUB_VS_FORMAT_H264_SIMULCAST)) ?	\
> +		(offsetof(struct usb_video_format_desc, u) +			\
> +		 sizeof(((struct usb_video_format_desc *)0)->u.h264)) :		\
> +	((fmt)->bDescriptorSubtype == UDESCSUB_VS_FORMAT_FRAME_BASED) ?		\
> +		(offsetof(struct usb_video_format_desc, u) +			\
> +		 sizeof(((struct usb_video_format_desc *)0)->u.fb)) :		\
> +	((fmt)->bDescriptorSubtype == UDESCSUB_VS_FORMAT_UNCOMPRESSED) ?	\
> +		(offsetof(struct usb_video_format_desc, u) +			\
> +		 sizeof(((struct usb_video_format_desc *)0)->u.uc)) :		\
> +	((fmt)->bDescriptorSubtype == UDESCSUB_VS_FORMAT_MJPEG) ?		\
> +		(offsetof(struct usb_video_format_desc, u) +			\
> +		 sizeof(((struct usb_video_format_desc *)0)->u.mjpeg)) :	\
> +	sizeof(struct usb_video_colorformat_desc)				\
> +	)
> +
>  } __packed;
>  
>  /*
>