On Fri, 27 Feb 2009, I wrote:
On Thu, 26 Feb 2009, Bjoern A. Zeeb wrote:
On Thu, 26 Feb 2009, Ed Schouten wrote:
Log:
Remove redundant assignment of `p'.
`p' is already initialized with `td->td_proc'. Because td is always
curthread, it is safe to initialize it without any locks.
Found by: LLVM's scan-build
Modified:
head/sys/kern/subr_prf.c
Modified: head/sys/kern/subr_prf.c
==============================================================================
--- head/sys/kern/subr_prf.c Thu Feb 26 12:06:46 2009 (r189065)
+++ head/sys/kern/subr_prf.c Thu Feb 26 12:12:34 2009 (r189066)
@@ -137,7 +137,6 @@ uprintf(const char *fmt, ...)
return (0);
sx_slock(&proctree_lock);
- p = td->td_proc;
PROC_LOCK(p);
if ((p->p_flag & P_CONTROLT) == 0) {
PROC_UNLOCK(p);
I think this one is wrong. You should probably have removed the
assignment from declaration time as we are checking for td != NULL
just above that so it could possibly be a NULL pointer deref in the
initial assigment or the NULL check is redundant.
Almost all of them are wrong, since they keep the assignment with the
style bug (initialization in declaration) and remove the one without
the style bug. In libkern this also increases the divergence from the
libc versions, since the libc versions are missing the style bug.
However, this one is just a style bug, not a runtime bug. Since td is
always curthread, it not only doesn't need locking; it is guaranteed
to be non-NULL and the check that it is NULL is just garbage. My very
old fix for style bugs in uprintf() removes the check as well as removing
the correct initialization:
% Index: subr_prf.c
% ===================================================================
% RCS file: /home/ncvs/src/sys/kern/subr_prf.c,v
% retrieving revision 1.111
% diff -u -2 -r1.111 subr_prf.c
% --- subr_prf.c 18 Jun 2004 20:12:42 -0000 1.111
% +++ subr_prf.c 20 Jun 2004 04:40:46 -0000
% @@ -123,13 +119,13 @@
% uprintf(const char *fmt, ...)
% {
% - struct thread *td = curthread;
% - struct proc *p = td->td_proc;
% va_list ap;
% struct putchar_arg pca;
% + struct proc *p;
% + struct thread *td;
% int retval;
%
% - if (td == NULL || td == PCPU_GET(idlethread))
% + td = curthread;
% + if (td == PCPU_GET(idlethread))
% return (0);
% -
% p = td->td_proc;
% PROC_LOCK(p);
% @@ -148,5 +144,4 @@
% retval = kvprintf(fmt, putchar, &pca, 10, ap);
% va_end(ap);
% -
% return (retval);
% }
Other style bugs fixed here:
- declarations were disordered (td before p to facilitate the style bugs
in the initializations, and pointers before structs for no reason)
- another initialization in declaration
- extra blank lines
Bruce
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
