linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: SF Markus Elfring <elfring@users.sourceforge.net>
To: Hans de Goede <hdegoede@redhat.com>, linux-iio@vger.kernel.org
Cc: Hartmut Knaack <knaack.h@gmx.de>,
	Jonathan Cameron <jic23@kernel.org>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
	LKML <linux-kernel@vger.kernel.org>,
	kernel-janitors@vger.kernel.org
Subject: Re: iio/accel/bmc150: Improve unlocking of a mutex in two functions
Date: Wed, 25 Oct 2017 18:58:10 +0200	[thread overview]
Message-ID: <7a76d804-797c-8a7f-a755-9e42f9157287@users.sourceforge.net> (raw)
In-Reply-To: <6a2c614a-4d0d-f2d2-1689-4c67a2fc3eea@redhat.com>

> If that is the only unlock in the function, then it is probably
> best to keep things as is. In general gotos are considered
> better then multiple unlocks, but not having either is
> even better.

Thanks for your quick feedback.


>> How do you think about to use the following code variant then?
>>
>>     if (!ret)
>>         ret = IIO_VAL_INT;
> 
> 
> I believe the goto unlock variant and setting ret = IIO_VAL_INT;
> directly above the unlock label variant is better,

I would prefer the approach above so that an extra goto statement
could also be avoided before.


> because that way the error handling is consistent between all steps
> and if another step is later added at the end, the last step will
> not require modification.

Do any more contributors insist on such a code layout?


>> How long should I wait for corresponding feedback before another small
>> source code adjustment will be appropriate?
> 
> That is hard to say.

I am just curious on how we can achieve progress here.


> I usually just do a new version when I've time,

This is generally fine.


> seldomly someone complains I should have waited longer for feedback
> (when I'm quite quick) but usually sending out a new version as soon
> as you've time to work on a new version is best, since if you wait
> you may then not have time for the entire next week or so, at least
> that is my experience :)  There is really no clear rule here.

I asked also because other well-known contributors showed strong
reactions for this change pattern (with the help of a script
for the semantic patch language).
Would you care for similar updates in source files like the following?

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/iio/accel/kxcjk-1013.c?id=1540d0106bcbc4e52013d759a0a0752ae7b4a09d#n760
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/iio/accel/stk8312.c?id=36ef71cae353f88fd6e095e2aaa3e5953af1685d#n432
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/iio/adc/palmas_gpadc.c?id=36ef71cae353f88fd6e095e2aaa3e5953af1685d#n304

Regards,
Markus

  reply	other threads:[~2017-10-25 16:58 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-25 14:33 [PATCH] iio/accel/bmc150: Improve unlocking of a mutex in two functions SF Markus Elfring
2017-10-25 15:57 ` Hans de Goede
2017-10-25 16:15   ` SF Markus Elfring
2017-10-25 16:22     ` Hans de Goede
2017-10-25 16:58       ` SF Markus Elfring [this message]
2017-10-25 17:28         ` Hans de Goede
2017-10-25 18:07           ` SF Markus Elfring
2017-10-26 15:46             ` Jonathan Cameron
2017-10-26 15:51       ` [PATCH] " Jonathan Cameron
2017-10-26 16:04         ` Jonathan Cameron

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=7a76d804-797c-8a7f-a755-9e42f9157287@users.sourceforge.net \
    --to=elfring@users.sourceforge.net \
    --cc=hdegoede@redhat.com \
    --cc=jic23@kernel.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=knaack.h@gmx.de \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=srinivas.pandruvada@linux.intel.com \
    /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;
as well as URLs for NNTP newsgroup(s).