Thread (7 messages) 7 messages, 2 authors, 2018-06-22

Re: [PATCH v2 2/2] rusage: allow 64-bit times ru_utime/ru_stime

From: Ingo Molnar <mingo@kernel.org>
Date: 2018-06-21 15:49:24
Also in: linux-api, linux-arch, lkml

(belated reply)

* Arnd Bergmann [off-list ref] wrote:
+int put_compat_rusage_time64(const struct __kernel_rusage *r,
+			     struct compat_rusage_time64 __user *ru)
+{
+	struct compat_rusage_time64 r32;
+	memset(&r32, 0, sizeof(r32));
+	r32.ru_utime.tv_sec = r->ru_utime.tv_sec;
+	r32.ru_utime.tv_usec = r->ru_utime.tv_usec;
+	r32.ru_stime.tv_sec = r->ru_stime.tv_sec;
+	r32.ru_stime.tv_usec = r->ru_stime.tv_usec;
+	r32.ru_maxrss = r->ru_maxrss;
+	r32.ru_ixrss = r->ru_ixrss;
+	r32.ru_idrss = r->ru_idrss;
+	r32.ru_isrss = r->ru_isrss;
+	r32.ru_minflt = r->ru_minflt;
+	r32.ru_majflt = r->ru_majflt;
+	r32.ru_nswap = r->ru_nswap;
+	r32.ru_inblock = r->ru_inblock;
+	r32.ru_oublock = r->ru_oublock;
+	r32.ru_msgsnd = r->ru_msgsnd;
+	r32.ru_msgrcv = r->ru_msgrcv;
+	r32.ru_nsignals = r->ru_nsignals;
+	r32.ru_nvcsw = r->ru_nvcsw;
+	r32.ru_nivcsw = r->ru_nivcsw;
Could you please vertically align the right side of the initialization as well? 
Much easier to check at a glance.
+	user_access_begin();
+	unsafe_put_user(signo, &infop->si_signo, Efault);
+	unsafe_put_user(0, &infop->si_errno, Efault);
+	unsafe_put_user(info.cause, &infop->si_code, Efault);
+	unsafe_put_user(info.pid, &infop->si_pid, Efault);
+	unsafe_put_user(info.uid, &infop->si_uid, Efault);
+	unsafe_put_user(info.status, &infop->si_status, Efault);
+	user_access_end();
This too would look nicer the following way:
+	user_access_begin();
+	unsafe_put_user(signo,		&infop->si_signo,	Efault);
+	unsafe_put_user(0,		&infop->si_errno,	Efault);
+	unsafe_put_user(info.cause,	&infop->si_code,	Efault);
+	unsafe_put_user(info.pid,	&infop->si_pid,		Efault);
+	unsafe_put_user(info.uid,	&infop->si_uid,		Efault);
+	unsafe_put_user(info.status,	&infop->si_status,	Efault);
+	user_access_end();
Which tabulated form made me notice the info.cause / si_code asymmetry - and a 
brief check of the source shows that it's correct. No way would I have noticed it 
in the jumbled up form above, so I think aligning such mass-initializations makes 
sense.
+	memset(&r, 0, sizeof(r));
+	r.ru_utime.tv_sec = rk->ru_utime.tv_sec;
+	r.ru_utime.tv_usec = rk->ru_utime.tv_usec;
+	r.ru_stime.tv_sec = rk->ru_stime.tv_sec;
+	r.ru_stime.tv_usec = rk->ru_stime.tv_usec;
+	r.ru_maxrss = rk->ru_maxrss;
+	r.ru_ixrss = rk->ru_ixrss;
+	r.ru_idrss = rk->ru_idrss;
+	r.ru_isrss = rk->ru_isrss;
+	r.ru_minflt = rk->ru_minflt;
+	r.ru_majflt = rk->ru_majflt;
+	r.ru_nswap = rk->ru_nswap;
+	r.ru_inblock = rk->ru_inblock;
+	r.ru_oublock = rk->ru_oublock;
+	r.ru_msgsnd = rk->ru_msgsnd;
+	r.ru_msgrcv = rk->ru_msgrcv;
+	r.ru_nsignals = rk->ru_nsignals;
+	r.ru_nvcsw = rk->ru_nvcsw;
+	r.ru_nivcsw = rk->ru_nivcsw;
Ditto.

Thanks,

	Ingo
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help