| Message ID | 20241108142310.19794-2-isaac.scott@ideasonboard.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Fix Sonix Technology MJPEG streams | expand |
Hi Isaac, Thank you for the patch. On Fri, Nov 08, 2024 at 02:23:08PM +0000, Isaac Scott wrote: > The uvc_video_decode_start function returns the first byte of the header, > which is in fact the length of the header. Improve readability by using an > appropriately named variable. > > Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com> > --- > drivers/media/usb/uvc/uvc_video.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c > index e00f38dd07d9..2fb9f2b59afc 100644 > --- a/drivers/media/usb/uvc/uvc_video.c > +++ b/drivers/media/usb/uvc/uvc_video.c > @@ -1117,6 +1117,7 @@ static int uvc_video_decode_start(struct uvc_streaming *stream, > struct uvc_buffer *buf, const u8 *data, int len) > { > u8 fid; > + u8 header_len = data[0]; Is there a guarantee at this point, before the sanity checks below, that len is not zero, that is, that it's safe to read data[0] ? > > /* > * Sanity checks: > @@ -1212,7 +1213,7 @@ static int uvc_video_decode_start(struct uvc_streaming *stream, > > stream->last_fid = fid; > > - return data[0]; > + return header_len; > } > > static inline enum dma_data_direction uvc_stream_dir(
Hi Isaac nit: If you have to redo the set I would recommend to squash this patch to the next one. On Fri, 8 Nov 2024 at 16:43, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Isaac, > > Thank you for the patch. > > On Fri, Nov 08, 2024 at 02:23:08PM +0000, Isaac Scott wrote: > > The uvc_video_decode_start function returns the first byte of the header, > > which is in fact the length of the header. Improve readability by using an > > appropriately named variable. > > > > Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com> > > --- > > drivers/media/usb/uvc/uvc_video.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c > > index e00f38dd07d9..2fb9f2b59afc 100644 > > --- a/drivers/media/usb/uvc/uvc_video.c > > +++ b/drivers/media/usb/uvc/uvc_video.c > > @@ -1117,6 +1117,7 @@ static int uvc_video_decode_start(struct uvc_streaming *stream, > > struct uvc_buffer *buf, const u8 *data, int len) > > { > > u8 fid; > > + u8 header_len = data[0]; > > Is there a guarantee at this point, before the sanity checks below, that > len is not zero, that is, that it's safe to read data[0] ? > > > > > /* > > * Sanity checks: > > @@ -1212,7 +1213,7 @@ static int uvc_video_decode_start(struct uvc_streaming *stream, > > > > stream->last_fid = fid; > > > > - return data[0]; > > + return header_len; > > } > > > > static inline enum dma_data_direction uvc_stream_dir( > > -- > Regards, > > Laurent Pinchart >
diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index e00f38dd07d9..2fb9f2b59afc 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -1117,6 +1117,7 @@ static int uvc_video_decode_start(struct uvc_streaming *stream, struct uvc_buffer *buf, const u8 *data, int len) { u8 fid; + u8 header_len = data[0]; /* * Sanity checks: @@ -1212,7 +1213,7 @@ static int uvc_video_decode_start(struct uvc_streaming *stream, stream->last_fid = fid; - return data[0]; + return header_len; } static inline enum dma_data_direction uvc_stream_dir(
The uvc_video_decode_start function returns the first byte of the header, which is in fact the length of the header. Improve readability by using an appropriately named variable. Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com> --- drivers/media/usb/uvc/uvc_video.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)