linux-kernel - Re: [PATCH] watchdog: make imx2_wdt report boot status correctly

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120216141707.GA8647@curry>
Date:	16 Feb 2012 14:17:07 +0000
From:	"Oskar Schirmer" <oskar@...ra.com>
To:	"Wolfram Sang" <w.sang@...gutronix.de>
Cc:	"Oskar Schirmer" <oskar@...ra.com>, wim@...ana.be,
	s.hauer@...gutronix.de, linux-watchdog@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] watchdog: make imx2_wdt report boot status correctly

Hi Wolfram,

On Thu, Feb 16, 2012 at 14:35:10 +0100, Wolfram Sang wrote:
> besides this minor thing (which may be just personal taste)...
> 
> > +	case WDIOC_GETBOOTSTATUS:
> > +		val = __raw_readw(imx2_wdt.base + IMX2_WDT_WRSR);
> > +		new_value = 0;
> > +		if (val & IMX2_WDT_WRSR_TOUT)
> > +			new_value = WDIOF_CARDRESET;
> 
> I'd go for this to save some lines:
> 		new_value = val & IMX2_WDT_WRSR_TOUT ? WDIOF_CARDRESET : 0;

A good alternative for sure.

Seen from the perspective of code compactness ("save some lines"),
one could try to be consequent here, and save the "val" variable
altogether, ending up with some "new_value = __raw_readw ... & ... ? ... : 0;"
But eventually there might be code readability issues, so it's
a good idea to keep the balance.

An argument for the original, longer version might be it is easier
to extend, when more flags need to be handled, like
"else if (val & ...) new_value = ...", while nested conditional
expressions would most likely become quite complex soon.

Actually, my personal taste doesn't show a preference for
one version or the other, so I'm ok with Your proposal, too.

  Oskar
--
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/