Re: [RFC PATCH v2 03/11] time: Add rtc_tm_to_time64() safe version(using time64_t)

[Thomas Gleixner]

On Fri, 31 Oct 2014, pang.xunlei wrote:
> On 30 October 2014 21:55, Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
> > What's wrong with changing it to:
> >
> > time64_t rtc_tm_to_time64(struct rtc_time *tm)
> > {
> >         return mktime64(......);
> > }
> 
> Thank you for reminding me of this, and also a valuable principle when
> doing things.
> 
> I wanna change it to:
> void rtc_tm_to_time64(struct rtc_time *tm, time64_t *time)
> {
>     time = mktime64(......);
> }
> 
> Just keep it the similar format as rtc_time64_to_tm(), is this acceptable?

Why on earth want you do that? Just because it looks the same or what?

Come on, it's not that hard and this is not the CS 'Java for dummies'
course where you get an A for implementing 'looks the same' nonsense.

For rtc_time64_to_tm() you really want to hand in the pointer for the
result because returning the full structure is overkill as it involves
heavy stack operations.

But for rtc_tm_to_time64() this is a different story. All relevant
32bit architectures can deal with a u64 return value very well and in
most cases it creates better code.

Just do the simple experiment of implementing both, converting a dozen
drivers and comparing the resulting assembler code on the call sites
and the function itself. That should give you an objective decision
criteria instead of "wanna change just because I feel it looks better".

Another valuable principle of doing things to remember, right?

Thanks,

	tglx






--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/