linux-kernel - Re: [PATCH v2] rtc: Add NXP PCF8523 support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 29 Nov 2012 07:41:53 +0100
From:	Thierry Reding <thierry.reding@...onic-design.de>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Alessandro Zummo <a.zummo@...ertech.it>,
	rtc-linux@...glegroups.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] rtc: Add NXP PCF8523 support

On Wed, Nov 28, 2012 at 03:31:58PM -0800, Andrew Morton wrote:
> On Wed, 28 Nov 2012 20:21:26 +0100
> Thierry Reding <thierry.reding@...onic-design.de> wrote:
> 
> > +	err = i2c_transfer(client->adapter, &msg, 1);
> > +	if (err < 0) {
> > +		/*
> > +		 * If the time cannot be set, restart the RTC anyway. Note
> > +		 * that errors are ignored if the RTC cannot be started so
> > +		 * that we have a chance to propagate the original error.
> > +		 */
> > +		pcf8523_start_rtc(client);
> > +		return err;
> > +	}
> > +
> > +	return pcf8523_start_rtc(client);
> 
> hm, well, that is of course equivalent to
> 
> 	err = i2c_transfer(client->adapter, &msg, 1);
> 	pcf8523_start_rtc(client);
> 	return err;

At the risk of sounding pedantic, it isn't quite the same thing. There
is a possibility that i2c_transfer() succeeds but pcf8523_start_rtc()
still fails. So in fact we catch errors in i2c_transfer() and from
starting the RTC in case the time was properly set. The only error we
might loose if if the RTC fails to start after the time cannot be set
but in that case there's probably not a whole lot we can do.

> but I suppose the code as proposed is clear, extensible, and not our
> worst-ever sin ;)

Yes, this is one of the cases where the gain in clarity justifies the
extra verbosity. =)

Thierry

Content of type "application/pgp-signature" skipped