[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <10422468.Ah80osK73i@avalon>
Date: Tue, 01 Apr 2014 17:24:14 +0200
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Anton Leontiev <bunder@...5.ru>
Cc: Mauro Carvalho Chehab <m.chehab@...sung.com>,
linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] [media] uvcvideo: Fix marking buffer erroneous in case of FID toggling
Hi Anton,
On Saturday 29 March 2014 09:28:02 Anton Leontiev wrote:
> 28.03.2014 20:12, Laurent Pinchart пишет:
> >>>> + * Set error flag for incomplete buffer.
> >>>> + */
> >>>> +static void uvc_buffer_check_bytesused(const struct uvc_streaming
> >>>> *const stream,
> >
> > No need for the second const keyword here.
> >
> > I would have used "uvc_video_" as a prefix, to be in sync with the
> > surrounding functions. What would you think of
> > uvc_video_validate_buffer() ?
> >
> >>>> + struct uvc_buffer *const buf)
> >
> > And no need for const at all here.
> >
> >>>> +{
> >>>> + if (buf->length != buf->bytesused &&
> >>>> + !(stream->cur_format->flags & UVC_FMT_FLAG_COMPRESSED))
> >
> > The indentation is wrong here, the ! on the second line should be aligned
> > to the first 'buf' of the first line.
> >
> > If you agree with these changes I can perform them while applying, there's
> > no need to resubmit the patch.
>
> Thank you for reviewing my first patch to Linux kernel.
Thank you for submitting it :-)
> I completely agree with your changes.
>
> Just want to ask why there is no need for the second 'const' after pointer
> character '*'? I thought it marks pointer itself as constant for type-
> checking opposite to first 'const', which marks memory it points to as
> constant for type-checking.
Your understanding is correct (as far as I know).
> I understand that the function is simple enough to verify it by hand but
> it's better to add more information for automatic checking.
>
> Is there any guidelines on 'const' keyword usage in Linux kernel code?
Marking the memory pointed to by the pointer as const ensures that the
function won't modify memory that the caller thought wouldn't be modified. It
thus serves as a contract between the caller and the callee, enforced by the
compiler.
Marking the pointer as const, on the other hand, only affects the function
implementation, without influencing its call contract. Its only use in this
case would be to prevent the function from modifying a pointer that the
function itself thought it shouldn't modify. While not useless in all cases,
this compile-time checked if usually not performed in the kernel, especially
for simple functions. I'm not aware of any explicit rule regarding const usage
though.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists