From: "Kim, HeungJun" <riverful.kim@samsung.com>
To: Sakari Ailus <sakari.ailus@iki.fi>
Cc: Hans Verkuil <hverkuil@xs4all.nl>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
linux-media@vger.kernel.org,
Hans Verkuil <hans.verkuil@cisco.com>
Subject: Re: [RFCv2 PATCH 08/11] v4l2-ctrls: simplify event subscription.
Date: Wed, 08 Jun 2011 19:17:53 +0900 [thread overview]
Message-ID: <4DEF4C51.3090302@samsung.com> (raw)
In-Reply-To: <20110606100534.GI6073@valkosipuli.localdomain>
2011-06-06 오후 7:05, Sakari Ailus 쓴 글:
> On Sat, Jun 04, 2011 at 12:28:04PM +0200, Hans Verkuil wrote:
>> On Friday, June 03, 2011 21:55:10 Laurent Pinchart wrote:
> [clip]
>>>> +{
>>>> + int ret = 0;
>>>> +
>>>> + if (!fh->events)
>>>> + ret = v4l2_event_init(fh);
>>>> + if (!ret)
>>>> + ret = v4l2_event_alloc(fh, n);
>>>> + if (!ret)
>>>> + ret = v4l2_event_subscribe(fh, sub);
>>>
>>> I tend to return errors when they occur instead of continuing to the end of
>>> the function. Handling errors on the spot makes code easier to read in my
>>> opinion, as I expect the main code flow to be the error-free path.
>>
>> Hmmm, I rather like the way the code looks in this particular case. But it;s
>> no big deal and I can change it.
>
> The M5MOLS driver uses this pattern extensively in I2C access error
> handling. I agree with Laurent in principle, but on the other hand I think
> using this pattern makes sense. The error handling takes much less code and
> the test for continuing always is "if (!ret)" it is relatively readable as
> well.
>
> I'm fine with either resolution.
Actually, this pattern was advice from Hans for the M-5MOLS reviews at past,
and it's very helpful and I satisfied mostly.
Also, I was testing or using both this usage and the other one for a while,
and as a result, I figured out which is good choice between continueing to the end
of the function and return imediately, depends on various situation.
Consequently, IMHO, both usgaes are fine, but if the checking factor like (!ret),
changes the other one like (!flag) not ret, then it looks better to return
immediately, or the other handling methods, not using continueing to check error
at the end.
Regards,
Heungjun Kim
next prev parent reply other threads:[~2011-06-08 10:17 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-25 13:33 [RFCv2 PATCH 00/11] Control Event Hans Verkuil
2011-05-25 13:33 ` [RFCv2 PATCH 01/11] v4l2-ctrls: introduce call_op define Hans Verkuil
2011-05-25 13:33 ` [RFCv2 PATCH 02/11] v4l2-ctrls: drivers should be able to ignore READ_ONLY and GRABBED flags Hans Verkuil
2011-05-25 13:33 ` [RFCv2 PATCH 03/11] v4l2-ioctl: add ctrl_handler to v4l2_fh Hans Verkuil
2011-05-25 13:33 ` [RFCv2 PATCH 04/11] v4l2-ctrls: Replace v4l2_ctrl_activate/grab with v4l2_ctrl_flags Hans Verkuil
2011-06-03 19:55 ` Laurent Pinchart
2011-06-04 10:35 ` Hans Verkuil
2011-05-25 13:33 ` [RFCv2 PATCH 05/11] v4l2-ctrls: add v4l2_fh pointer to the set control functions Hans Verkuil
2011-06-03 19:55 ` Laurent Pinchart
2011-06-04 10:31 ` Hans Verkuil
2011-05-25 13:33 ` [RFCv2 PATCH 06/11] vb2_poll: don't start DMA, leave that to the first read() Hans Verkuil
2011-05-25 13:33 ` [RFCv2 PATCH 07/11] v4l2-ctrls: add control events Hans Verkuil
2011-05-28 10:34 ` Sakari Ailus
2011-05-28 14:58 ` Hans Verkuil
2011-06-05 13:10 ` Sakari Ailus
2011-05-28 10:48 ` Sakari Ailus
2011-05-28 15:08 ` Hans Verkuil
2011-06-01 7:15 ` Sakari Ailus
2011-05-25 13:33 ` [RFCv2 PATCH 08/11] v4l2-ctrls: simplify event subscription Hans Verkuil
2011-06-03 19:55 ` Laurent Pinchart
2011-06-04 10:28 ` Hans Verkuil
2011-06-06 10:05 ` Sakari Ailus
2011-06-08 10:17 ` Kim, HeungJun [this message]
2011-05-25 13:33 ` [RFCv2 PATCH 09/11] vivi: support control events Hans Verkuil
2011-06-03 19:55 ` Laurent Pinchart
2011-06-04 10:21 ` Hans Verkuil
2011-05-25 13:33 ` [RFCv2 PATCH 10/11] V4L2 spec: document " Hans Verkuil
2011-05-25 13:33 ` [RFCv2 PATCH 11/11] v4l2-subdev: implement per-filehandle control handlers Hans Verkuil
2011-06-03 19:54 ` [RFCv2 PATCH 00/11] Control Event Laurent Pinchart
2011-06-04 10:19 ` Hans Verkuil
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4DEF4C51.3090302@samsung.com \
--to=riverful.kim@samsung.com \
--cc=hans.verkuil@cisco.com \
--cc=hverkuil@xs4all.nl \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
--cc=sakari.ailus@iki.fi \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox