Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 01 Sep 2001 00:43:25 -0700
From:      Peter Wemm <peter@wemm.org>
To:        Matt Dillon <dillon@FreeBSD.org>
Cc:        cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org
Subject:   Re: cvs commit: src/sys/kern subr_prof.c kern_ntptime.c kern_xxx.c 
Message-ID:  <20010901074325.28AE13807@overcee.netplex.com.au>
In-Reply-To: <200109010547.f815lwK20565@freefall.freebsd.org> 

next in thread | previous in thread | raw e-mail | index | archive | help
Matt Dillon wrote:
> dillon      2001/08/31 22:47:58 PDT
> 
>   Modified files:
>     sys/kern             subr_prof.c kern_ntptime.c kern_xxx.c 
>   Log:
>   Pushdown Giant for:	profil(), ntp_adjtime(), ogethostname(),
>   			osethostname(), ogethostid(), osethostid()

Thanks, but can you please try to stop going out of your way to cause
style(9) violations in previously compliant code?  eg:

+/*
+ * MPSAFE
+ */
 int
 oquota(p, uap)
        struct proc *p;
        struct oquota_args *uap;
 {
-
        return (ENOSYS);
 }
 #endif /* COMPAT_43 */

The specific section:

     static void
     usage()
     {
             /* Insert an empty line if the function has no local variables. */

Just about every one of these that you have touched like this has caused
a conflict with the kse diff because of the following example:

+/*
+ * MPSAFE (accept1() is MPSAFE)
+ */
 int
 oaccept(p, uap)
        struct proc *p;
        struct accept_args *uap;
 {
-
        return (accept1(p, uap, 1));
 }
 #endif /* COMPAT_OLDSOCK */
 
rcsmerge implodes when two adjacent deltas like this touch.  In KSE it was:

#ifdef COMPAT_OLDSOCK
int
oaccept(td, uap)
        struct thread *td;
        struct accept_args *uap;
{
                 
        return (accept1(td, uap, 1));
}
#endif /* COMPAT_OLDSOCK */

ie: your delta causes a conflict with the s/p/td/ change on the adjacent line.

You are also occasionally adding unnecesary blank lines out of sync with
the rest of the code, eg:

@@ -118,14 +121,17 @@
                int     protocol;
        } */ *uap;
 {
-       struct filedesc *fdp = p->p_fd;
+       struct filedesc *fdp;
        struct socket *so;
        struct file *fp;
        int fd, error;
 
+       mtx_lock(&Giant);
+
+       fdp = p->p_fd;
        error = falloc(p, &fp, &fd);
        if (error)
-               return (error);
+               goto done2;
        fhold(fp);
        error = socreate(uap->domain, &so, uap->type, uap->protocol, p);
        if (error) {

The added blank one after the mtx_lock() is the only one in the function
(socket()), for example.

Dont get me wrong, I appreciate the effort you are going to.  But it kinda
irks me when I have to resolve merge conflicts where style bugs were added
in the process. :-]

Cheers,
-Peter
--
Peter Wemm - peter@FreeBSD.org; peter@yahoo-inc.com; peter@netplex.com.au
"All of this is for nothing if we don't go to the stars" - JMS/B5


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010901074325.28AE13807>