Re: [PATCH][delayacct] Fix the timespec_sub() interface (was Re: [Patch 1/8] Setup)

[Balbir Singh <balbir@in.ibm.com>]

On Wed, May 10, 2006 at 03:57:16PM +0530, Balbir Singh wrote:
> On Wed, May 10, 2006 at 03:24:49AM -0700, Andrew Morton wrote:
> > Balbir Singh <balbir@in.ibm.com> wrote:
> > >
> > > Please find the updated patch, which changes the interface of timespec_sub()
> > > as suggested in the review comments
> > > 
> > > ...
> > >
> > >  /*
> > > - * sub = end - start, in normalized form
> > > + * sub = lhs - rhs, in normalized form
> > >   */
> > > -static inline void timespec_sub(struct timespec *start, struct timespec *end,
> > > -				struct timespec *sub)
> > > +static inline struct timespec timespec_sub(struct timespec *lhs,
> > > +						struct timespec *rhs)
> > >  {
> > 
> > I'd have thought that it would be more consistent and a saner interface to
> > use pass-by-value:
> > 
> > static inline struct timespec timespec_sub(struct timespec lhs,
> > 						struct timespec rhs)
> > 
> > It should generate the same code.
> > 
> > I mentioned this last time - did you choose to not do this for some reason,
> > or did it just slip past?
> 
> Sorry, that definitely slip past.
> 
> I'll send another update
>
Hi, Andrew,

Here is another attempt to do fix the interface.

	Thanks for the review,
	Balbir Singh,
	Linux Technology Center,
	IBM Software Labs



Changelog

1. Change the interface of timespec_sub() to return by value and rename
   the arguments. Use lhs,rhs instead of end,start.
2. Pass the arguments by value

Signed-off-by: Balbir Singh <balbir@in.ibm.com>
---

 include/linux/time.h |   12 +++++++-----
 kernel/delayacct.c   |    2 +-
 2 files changed, 8 insertions(+), 6 deletions(-)

diff -puN include/linux/time.h~timespec-sub-return-by-value include/linux/time.h
--- linux-2.6.17-rc3/include/linux/time.h~timespec-sub-return-by-value	2006-05-10 15:58:19.000000000 +0530
+++ linux-2.6.17-rc3-balbir/include/linux/time.h	2006-05-10 15:59:48.000000000 +0530
@@ -68,13 +68,15 @@ extern unsigned long mktime(const unsign
 extern void set_normalized_timespec(struct timespec *ts, time_t sec, long nsec);
 
 /*
- * sub = end - start, in normalized form
+ * sub = lhs - rhs, in normalized form
  */
-static inline void timespec_sub(struct timespec *start, struct timespec *end,
-				struct timespec *sub)
+static inline struct timespec timespec_sub(struct timespec lhs,
+						struct timespec rhs)
 {
-	set_normalized_timespec(sub, end->tv_sec - start->tv_sec,
-				end->tv_nsec - start->tv_nsec);
+	struct timespec ts_delta;
+	set_normalized_timespec(&ts_delta, lhs.tv_sec - rhs.tv_sec,
+				lhs.tv_nsec - rhs.tv_nsec);
+	return ts_delta;
 }
 
 /*
diff -puN kernel/delayacct.c~timespec-sub-return-by-value kernel/delayacct.c
--- linux-2.6.17-rc3/kernel/delayacct.c~timespec-sub-return-by-value	2006-05-10 15:58:19.000000000 +0530
+++ linux-2.6.17-rc3-balbir/kernel/delayacct.c	2006-05-10 16:00:30.000000000 +0530
@@ -74,7 +74,7 @@ static inline void delayacct_end(struct 
 	s64 ns;
 
 	do_posix_clock_monotonic_gettime(end);
-	timespec_sub(&ts, start, end);
+	ts = timespec_sub(*end, *start);
 	ns = timespec_to_ns(&ts);
 	if (ns < 0)
 		return;
_