Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Sun, 22 Sep 2019 21:05:42 +0200
From:   Alexandre Belloni <alexandre.belloni@bootlin.com>
To:     Pavel Machek <pavel@ucw.cz>
Cc:     Nick Crews <ncrews@chromium.org>, bleung@chromium.org,
        Alessandro Zummo <a.zummo@towertech.it>,
        enric.balletbo@collabora.com, linux-kernel@vger.kernel.org,
        dlaurie@chromium.org
Subject: Re: [PATCH v2 1/2] rtc: wilco-ec: Remove yday and wday calculations
Message-ID: <20190922190542.GC3185@piout.net>
References: <20190916181215.501-1-ncrews@chromium.org>
 <20190922161306.GA1999@bug>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20190922161306.GA1999@bug>
User-Agent: Mutt/1.12.1 (2019-06-15)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 22/09/2019 18:13:06+0200, Pavel Machek wrote:
> On Mon 2019-09-16 12:12:15, Nick Crews wrote:
> > The tm_yday and tm_wday fields are not used by userspace,
> > so since they aren't needed within the driver, don't
> > bother calculating them. This is especially needed since
> > the rtc_year_days() call was crashing if the HW returned
> > an invalid time.
> > 
> > Signed-off-by: Nick Crews <ncrews@chromium.org>
> > ---
> >  drivers/rtc/rtc-wilco-ec.c | 4 ----
> >  1 file changed, 4 deletions(-)
> > 
> > diff --git a/drivers/rtc/rtc-wilco-ec.c b/drivers/rtc/rtc-wilco-ec.c
> > index 8ad4c4e6d557..e84faa268caf 100644
> > --- a/drivers/rtc/rtc-wilco-ec.c
> > +++ b/drivers/rtc/rtc-wilco-ec.c
> > @@ -110,10 +110,6 @@ static int wilco_ec_rtc_read(struct device *dev, struct rtc_time *tm)
> >  	tm->tm_mday	= rtc.day;
> >  	tm->tm_mon	= rtc.month - 1;
> >  	tm->tm_year	= rtc.year + (rtc.century * 100) - 1900;
> > -	tm->tm_yday	= rtc_year_days(tm->tm_mday, tm->tm_mon, tm->tm_year);
> > -
> > -	/* Don't compute day of week, we don't need it. */
> > -	tm->tm_wday = -1;
> >  
> >  	return 0;
> 
> Are you sure? It would be bad to pass unititialized memory to userspace...
> 

This problem doesn't exist because userspace is passing the memory, not
the other way around.

> If userspace does not need those fields, why are they there?
> 

This is coming from struct tm, it is part of C89 but I think I was not
born when this decision was made. man rtc properly reports that those
fields are unused and no userspace tools are actually making use of
them. Nobody cares about the broken down representation of the time.
What is done is use the ioctl then mktime to have a UNIX timestamp.

"The mktime function ignores the specified contents of the tm_wday,
tm_yday, tm_gmtoff, and tm_zone members of the broken-down time
structure. It uses the values of the other components to determine the
calendar time; it’s permissible for these components to have
unnormalized values outside their normal ranges. The last thing that
mktime does is adjust the components of the brokentime structure,
including the members that were initially ignored."


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com