From: Kirill A. Korinsky <kirill@korins.ky>
Subject: Re: sys/uvideo: support pre UVC 1.5 devices
To: Marcus Glocker <marcus@nazgul.ch>
Cc: tech@openbsd.org
Date: Tue, 12 Aug 2025 18:30:50 +0200

On Tue, 12 Aug 2025 13:22:46 +0200,
Marcus Glocker <marcus@nazgul.ch> wrote:
> 
> > @@ -2044,13 +2045,18 @@ uvideo_vs_get_probe(struct uvideo_softc 
> >  
> >  	pc = (struct usb_video_probe_commit *)probe_data;
> >  
> > -	error = usbd_do_request(sc->sc_udev, &req, probe_data);
> > -	if (error) {
> > +	error = usbd_do_request_flags(sc->sc_udev, &req, probe_data, 0,
> > +	    &actlen, USBD_DEFAULT_TIMEOUT);
> 
> I'm OK if we accept short transfers here, but have you tried to use
> USBD_SHORT_XFER_OK as an usbd_do_request_flags() flag?  Then it shouldn't
> be required to make a specific error check on it.
> 

I haven't know that. Thank makes this diff much cleaner.

> > +	if (error != USBD_NORMAL_COMPLETION && error != USBD_SHORT_XFER) {
> >  		printf("%s: could not GET probe request: %s\n",
> >  		    DEVNAME(sc), usbd_errstr(error));
> >  		return (USBD_INVAL);
> >  	}
> >  
> > +	/* Different UVC version defines different length of a probe */
> > +	bzero(probe_data + actlen,
> > +	    sizeof(struct usb_video_probe_commit ) - actlen);
> > +
> 
> Why is it required to bzero probe_data here?  There is one use of
> uvideo_vs_get_probe() currently, and that always bzero's probe_data
> before:
> 
>         /* get probe */
>         bzero(probe_data, sizeof(probe_data));
>         error = uvideo_vs_get_probe(sc, probe_data, GET_CUR);
> 
> On a short transfer I wouldn't expect the rest of probe_data to contain
> garbage, but I have no device here to test that.
>

Yes, so do I.

From antoher hand clean it up, to avoid garbage from some devices seems not
that bad idea.


-- 
wbr, Kirill