From owner-svn-ports-head@freebsd.org  Fri Aug 26 16:15:03 2016
Return-Path: <owner-svn-ports-head@freebsd.org>
Delivered-To: svn-ports-head@mailman.ysv.freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org
 [IPv6:2001:1900:2254:206a::19:1])
 by mailman.ysv.freebsd.org (Postfix) with ESMTP id BA344B74656;
 Fri, 26 Aug 2016 16:15:03 +0000 (UTC) (envelope-from pi@FreeBSD.org)
Received: from fc.opsec.eu (fc.opsec.eu [IPv6:2001:14f8:200:4::4])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (Client did not present a certificate)
 by mx1.freebsd.org (Postfix) with ESMTPS id 67210BF5;
 Fri, 26 Aug 2016 16:14:57 +0000 (UTC) (envelope-from pi@FreeBSD.org)
Received: from pi by fc.opsec.eu with local (Exim 4.87 (FreeBSD))
 (envelope-from <pi@FreeBSD.org>)
 id 1bdJmP-000CbX-9g; Fri, 26 Aug 2016 18:14:53 +0200
Date: Fri, 26 Aug 2016 18:14:53 +0200
From: Kurt Jaeger <pi@FreeBSD.org>
To: Alex Kozlov <ak@FreeBSD.org>
Cc: Kurt Jaeger <pi@FreeBSD.org>, ports-committers@freebsd.org,
 svn-ports-all@freebsd.org, svn-ports-head@freebsd.org
Subject: Re: svn commit: r420921 - in head/x11/xdm: . files
Message-ID: <20160826161453.GX3061@fc.opsec.eu>
References: <201608261357.u7QDvDkt034771@repo.freebsd.org>
 <20160826145208.GA21079@ravenloft.kiev.ua>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20160826145208.GA21079@ravenloft.kiev.ua>
X-BeenThere: svn-ports-head@freebsd.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: SVN commit messages for the ports tree for head
 <svn-ports-head.freebsd.org>
List-Unsubscribe: <https://lists.freebsd.org/mailman/options/svn-ports-head>, 
 <mailto:svn-ports-head-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/svn-ports-head/>
List-Post: <mailto:svn-ports-head@freebsd.org>
List-Help: <mailto:svn-ports-head-request@freebsd.org?subject=help>
List-Subscribe: <https://lists.freebsd.org/mailman/listinfo/svn-ports-head>,
 <mailto:svn-ports-head-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Fri, 26 Aug 2016 16:15:03 -0000

Hi!

> >   x11/xdm: Use MIT-MAGIC-COOKIE-1 by default
[...]
> >   - pet portlint for the patches
> > +-DisplayManager.willing:		SU nobody -s /bin/sh -c XDMSCRIPTDIR/Xwilling
> > ++DisplayManager.keyFile:		/usr/local/lib/X11/xdm/xdm-keys

> Are you sure that replacing XDMCONFIGDIR with /usr/local/lib/X11/xdm/
> is a good idea?
> Same with XDMSCRIPTDIR.

No, it was a bad idea, but I missed that. Fixed.

> > +--- greeter/greet.c.orig	2011-09-25 07:35:47 UTC
> > ++++ greeter/greet.c
> > +@@ -639,6 +639,7 @@ greet_user_rtn GreetUser(
> >       }
> >       DeleteXloginResources (d, *dpy);
> >       CloseGreet (d);
> > @@ -8,9 +8,9 @@
> >       Debug ("Greet loop finished\n");
> >       /*
> >        * Run system-wide initialization file
> > -@@ -697,6 +704,8 @@ static int pamconv(int num_msg,
> > -     m = *msg;
> > -     r = *response;
> > +@@ -729,6 +730,8 @@ static int pamconv(int num_msg,
> > + 	goto pam_error;
> > +     }
> >   
> >  +    if (login == NULL) goto pam_error;
> >  +

> You're not only 'pet portlint', but also modified patches, which
> was not mentioned in the commit message nor in the pr.

The old patch was no longer changing all those things that needed
to change, but still applied. A make makepatch generated a
different patch, but the change was idempotent.

In the code, there was this:

    if (login == NULL) {
        status = PAM_CONV_ERR;
        goto pam_error;
    }

and the patch added an additional

    if (login == NULL) goto pam_error;

after the first part, which is no functional or other change.

I missed that, too. I'll commit a minimised patch for patch-greeter-greet.c.

> > Modified: head/x11/xdm/files/patch-xdm_session.c
> > ==============================================================================
> > --- head/x11/xdm/files/patch-xdm_session.c	Fri Aug 26 13:49:02 2016	(r420920)
> > +++ head/x11/xdm/files/patch-xdm_session.c	Fri Aug 26 13:57:13 2016	(r420921)
> > @@ -1,14 +1,14 @@
> > ---- xdm/session.c.orig	Sun Jun  3 22:49:51 2007
> > -+++ xdm/session.c	Sun Jun  3 22:56:06 2007
> > -@@ -543,6 +543,7 @@
> > +--- xdm/session.c.orig	2011-09-25 07:35:47 UTC
> > ++++ xdm/session.c
> > +@@ -575,6 +575,7 @@ StartClient (
> >       pid_t	pid;
> > - #ifdef HAS_SETUSERCONTEXT
> > + #ifdef HAVE_SETUSERCONTEXT
> >       struct passwd* pwd;
> >  +    extern char **environ;
> >   #endif
> >   #ifdef USE_PAM
> >       pam_handle_t *pamh = thepamh ();
> > -@@ -657,6 +660,8 @@
> > +@@ -695,6 +696,8 @@ StartClient (
> >   	 * Set the user's credentials: uid, gid, groups,
> >   	 * environment variables, resource limits, and umask.
> >   	 */
> > @@ -17,11 +17,11 @@
> >   	pwd = getpwnam(name);
> >   	if (pwd) {
> >   	    if (setusercontext(NULL, pwd, pwd->pw_uid, LOGIN_SETALL) < 0) {
> > -@@ -664,6 +669,7 @@
> > - 		    errno);
> > +@@ -702,6 +705,7 @@ StartClient (
> > + 			  name, _SysErrorMsg (errno));
> >   		return (0);
> >   	    }
> >  +	    verify->userEnviron = environ;
> >   	    endpwent();
> >   	} else {
> > - 	    LogError ("getpwnam for \"%s\" failed, errno=%d\n", name, errno);
> > + 	    LogError ("getpwnam for \"%s\" failed: %s\n",

The old patch was for a slightly different session.c,
but still applied. The changes to that patch are valid.

I missed those changes, too. But: There is no need for a different patch.

-- 
pi@FreeBSD.org         +49 171 3101372                4 years to go !