svn commit: r272752 - releng/10.1/sys/kern

[dteske at FreeBSD.org]

> -----Original Message-----
> From: owner-src-committers at freebsd.org [mailto:owner-src-
> committers at freebsd.org] On Behalf Of Neel Natu
> Sent: Wednesday, October 8, 2014 10:47 PM
> To: dteske at freebsd.org
> Cc: Neel Natu; src-committers at freebsd.org; svn-src-all at freebsd.org; svn-
> src-releng at freebsd.org
> Subject: Re: svn commit: r272752 - releng/10.1/sys/kern
> 
> Hi Devin,
> 
> On Wed, Oct 8, 2014 at 9:53 PM,  <dteske at freebsd.org> wrote:
> >
> >
> >> -----Original Message-----
> >> From: owner-src-committers at freebsd.org [mailto:owner-src-
> >> committers at freebsd.org] On Behalf Of Neel Natu
> >> Sent: Wednesday, October 8, 2014 8:39 AM
> >> To: src-committers at freebsd.org; svn-src-all at freebsd.org; svn-src-
> >> releng at freebsd.org
> >> Subject: svn commit: r272752 - releng/10.1/sys/kern
> >>
> >> Author: neel
> >> Date: Wed Oct  8 15:39:24 2014
> >> New Revision: 272752
> >> URL: https://svnweb.freebsd.org/changeset/base/272752
> >>
> >> Log:
> >>   MFC r272270:
> >
> > I hate to pick nits, but I believe this revision (272752 in releng/10.1)
> > should (I suggest; deferring to re@ for final prognosis) perhaps have
> > _not_ been an MFC from head (r272270; as was performed) but
> > perhaps have instead been MFS from stable/10 (r272726).
> >
> 
> The svn command used to do the merge was:
> # svn merge -c 272726 ^/stable/10 releng/10.1
> 
> This is exactly as per the SVN merge guidelines into releng as documented
> here:
> https://www.freebsd.org/doc/en/articles/committers-guide/subversion-
> primer.html
> 

Hi Neel,

Nowhere in that document does it describe "MFS".
I'm happy you did the merge as it was supposed to be done.
However, your commit message is inaccurate.

> The commit message used "MFC r272270" because that was the origin of
> the change and has the full details about the patch.
> 

I don't care if the subversion primer doesn't talk about "MFS" versus
"MFC", but it is technically inaccurate to call your commit an "MFC [of]
r272270" because it is in-fact (as you admit) an MFS of r272726 -- which
is indeed indicated by mergeinfo.

Making someone look up the mergeinfo because the commit message
is a inaccurate doesn't make for an efficient/predictable setup.
-- 
Cheers,
Devin

P.S. If I was really on your case, I'd insist that you had put "MFS10 r#"
but in all reality, "MFS r#" would have been perfectly acceptable where
"MFC r#" is clearly just plain wrong and misleading.

> best
> Neel
> 
> > The nit being that mergeinfo now shows (unnaturally) that things
> > flowed from head -> stable / head -> releng versus
> > head -> stable -> releng as I suggest would have been cleaner for
> > historical analysis.
> > --
> > Cheers,
> > Devin
> >
> >>
> >>   tty_rel_free() can be called more than once for the same tty so make
> sure
> >>   that the tty is dequeued from 'tty_list' only the first time.
> >>
> >>   Approved by:        re (glebius)
> >>
> >> Modified:
> >>   releng/10.1/sys/kern/tty.c
> >> Directory Properties:
> >>   releng/10.1/   (props changed)
> >>
> >> Modified: releng/10.1/sys/kern/tty.c
> >>
> ==========================================================
> >> ====================
> >> --- releng/10.1/sys/kern/tty.c        Wed Oct  8 15:30:59 2014        (r272751)
> >> +++ releng/10.1/sys/kern/tty.c        Wed Oct  8 15:39:24 2014        (r272752)
> >> @@ -1055,13 +1055,13 @@ tty_rel_free(struct tty *tp)
> >>       tp->t_dev = NULL;
> >>       tty_unlock(tp);
> >>
> >> -     sx_xlock(&tty_list_sx);
> >> -     TAILQ_REMOVE(&tty_list, tp, t_list);
> >> -     tty_list_count--;
> >> -     sx_xunlock(&tty_list_sx);
> >> -
> >> -     if (dev != NULL)
> >> +     if (dev != NULL) {
> >> +             sx_xlock(&tty_list_sx);
> >> +             TAILQ_REMOVE(&tty_list, tp, t_list);
> >> +             tty_list_count--;
> >> +             sx_xunlock(&tty_list_sx);
> >>               destroy_dev_sched_cb(dev, tty_dealloc, tp);
> >> +     }
> >>  }
> >>
> >>  void
> >
> >