[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [E-devel] New patch for entranced and entrance



Hi, here's another patch for entrance.

OK, I've done this acccording to what raster told me, I hope  I understood these things properly.

1) A shell should be run, so the users's profile is loaded. After thinking about it, this is a great idea indeed, this way if I have a ~/bin in my PATH my wm can run those applications directly (no strings attatched) and it's the right thing to do.

2) The first attempted shell should be the users's shell. This is almost guaranteed to have -l. We only try to run this if the shell isn't "".

3) We run /bin/sh -l. If the users's shell with a '-l' didn't work (maybe because it was uninstalled or something) we may get to this point.

4) If the above didn't work, we log a warning message and try to run "/bin/sh" without -l anyway. Note that the users's PATH, will probably not be setup properly in this case (at least for me programs in /usr/local/bin weren't available with the run thingie and my eapps in the favorite applications menu were missing)

Oh, btw does the run thingie have a name so I can stop calling it the run thingie? :)

5) If the above failed as well (the user's shell is uninstalled and /bin/sh was linking to it or whatever) we log an error message and cry :((

Some things that should me noted about my patch
1) As before, X isn't started through a shell anymore. The cmdline is parsed and executed directly, since this was desired.

2) I've cleaned up the code in entrance_session_start_user_session if a few ways.
 a) there was a 'shell' variable which wasn't getting used. I found a good use for it.
  b) the current code is structured in a weird way imho. The call to execl() is outside the fork switch, so, somehow, it would call the session in the child pid  and recall entrance_login in the parent pid.

Why would we want to do that? X will restart and entrance_login will get called again anyway, when the program exits.

In my patch, the parent calls wait(), holding its pants on until the child's session ends. No more sleep(), less code in the function and things look a bit more normal().

3) In _entrance_session_execute_in_shell, each shell is called inside a fork, and waited for by the parent process. This is done so we can catch errors like 'sh: Illegal option -l'. Is this in any way wrong?

4)  Oh, and ecore_x_shutdown(); and ecore_shutdown() are being called now, just like the TODO says.

5) I've tested it in three ways.
 - working user shell
 - user shell not working, sh is a login shell
 - user shell not working, sh is not a login shell.
Everything went well in all these circumstances.


That's about it. Is this one any good?
Cheers,
Eugen.

PS: Congrats Essien for changing the session handling. It works great now.


On 8/29/06, The Rasterman Carsten Haitzler <raster@rasterman.com> wrote:
On Tue, 29 Aug 2006 06:12:47 +0200 Sebastian Dransfeld <sebastid@stud.ntnu.no>
babbled:

> Carsten Haitzler (The Rasterman) wrote:
> > On Mon, 28 Aug 2006 16:55:19 +0200 Sebastian Dransfeld
> > <sebastid@stud.ntnu.no> babbled:
> >
> >> Eugen Minciu wrote:
> >>> On Mon, 28 Aug 2006 14:29:08 +0100
> >>> Essien Ita Essien < essien@wazobialinux.com> wrote:
> >>>
> >>> Well .. then maybe we shouldn't clearenv() in the first place? We may not
> >>> need to, since setenv(x,y,1) is called, which overwrites the var's
> >>> contents anyway. I didn't want to try it before because I suspected
> >>> clearenv() was there for good reasons (and it feels right, too).
> >>>
> >>> However, it's strange you should mention this .. there is:
> >>> entrance_auth_setup_environment(Entrance_Auth * e, const char *display,
> >>> const char *path) so the display is sent as a paramter and it _should be_
> >>> set by the function itself.
> >>>
> >>> Could you investigate this a bit further? I'm willing to try not clearenv
> >>> ()
> >>> - ing but it may introduce some even subtler (and weirder) problems. If
> >>> you think it shouldn't though I'll try it out later on tonight, when I've
> >>> some free time on my hands.
> >> clearenv must stay. The user must not inherit any environment from
> >> entrance.
> >
> > well- it does NEED to inherit DISPLAY... for starters. :) the problem is
> > actually entrance executing enlightenment directly. no other dm does this as
> > best i know. every login session is first executed by a shell which then
> > executes the wm or session manager etc. thus the behavior issues. wm's like
> > e expect to have a users environment already loaded by the time they run.
> > (ie users $PATH is set and all the other goodies a user wants in their
> > SHELL)
>
> DISPLAY is set by entrance for the user, so I would not call it inherit
> from entrance. clearenv first, then set user environment (DISPLAY++),
> then launch user session in a login shell.

well the only way to set an env var in  child process is to set it before the
process runs (either in the parent env or explicitly in the exec) :)but e
doesnt magically set it itself :)

--
------------- Codito, ergo sum - "I code, therefore I am" --------------
The Rasterman (Carsten Haitzler)    raster@rasterman.com
裸好多
Tokyo, Japan (東京 日本)

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel


Index: src/client/entrance_session.c
===================================================================
RCS file: /var/cvs/e/e17/apps/entrance/src/client/entrance_session.c,v
retrieving revision 1.85
diff -u -r1.85 entrance_session.c
--- src/client/entrance_session.c	28 Aug 2006 04:12:44 -0000	1.85
+++ src/client/entrance_session.c	30 Aug 2006 08:44:28 -0000
@@ -25,6 +25,7 @@
 extern void user_unselected_cb(void *data, Evas_Object * o,
                                const char *emission, const char *source);
 static void _entrance_session_user_list_fix(Entrance_Session * e);
+static void _entrance_session_execute_in_shell(char *user,char *shell,char *buf);
 
 /**
  * entrance_session_new: allocate a new  Entrance_Session
@@ -379,6 +380,7 @@
    pid_t pid;
    char buf[PATH_MAX];
    char *shell = NULL;
+   char *user = NULL;
    struct passwd *pwent = NULL;
    Entrance_X_Session *exs = NULL;
 
@@ -417,7 +419,9 @@
       e->ee = NULL;
    }
 
-   syslog(LOG_NOTICE, "Starting session for user \"%s\".", e->auth->user);
+   user = strdup(e->auth->user);
+
+   syslog(LOG_NOTICE, "Starting session for user \"%s\".", user);
 
 #ifdef HAVE_PAM
    if (e->config->auth == ENTRANCE_USE_PAM)
@@ -464,41 +468,27 @@
         if (setuid(pwent->pw_uid))
            syslog(LOG_CRIT, "Unable to set user id.");
         shell = strdup(pwent->pw_shell);
+
+        /* replace this process with a clean small one that just waits for its */
+        /* child to exit.. passed on the cmd-line */
+
+        _entrance_session_execute_in_shell(user,shell,buf);
         break;
      case -1:
         syslog(LOG_INFO, "FORK FAILED, UH OH");
         exit(0);
      default:
         syslog(LOG_NOTICE, "Replacing Entrance with simple login program to wait for session end.");
-#ifdef HAVE_PAM
-        if (e->config->auth == ENTRANCE_USE_PAM)
-        {
-           snprintf(buf, sizeof(buf), "%s/%s/entrance_login %i %s %s",
-                    PACKAGE_LIB_DIR, PACKAGE, (int) pid, pwent->pw_name, 
-		    e->display);
-        }
-        else
-#endif
-        {
-           snprintf(buf, sizeof(buf), "%s/%s/entrance_login %i",
-                    PACKAGE_LIB_DIR, PACKAGE, (int) pid);
-        }
-        shell = strdup("/bin/sh");
-        /* this bypasses a race condition where entrance loses its x
-           connection before the wm gets it and x goes and resets itself */
-        sleep(10);
-        /*
-         * FIXME These should be called!
+	wait(NULL); 
         ecore_x_shutdown();
         ecore_shutdown();
-        */
         break;
    }
    struct_passwd_free(pwent);
    entrance_session_free(e);
-   /* replace this process with a clean small one that just waits for its */
-   /* child to exit.. passed on the cmd-line */
-   execl("/bin/sh", "/bin/sh", "-l", "-c", buf, NULL);
+   if (shell) free(shell);
+   if (user) free(user);
+   if (buf) free(buf);
 }
 
 
@@ -799,3 +789,62 @@
       }
    }
 }
+
+static void
+_entrance_session_execute_in_shell(char *user,char *shell,char *buf)
+{
+   int pid;
+   int status;
+   int res=0;
+
+   /* If the user's passwd entry has a shell try to run it in login mode */
+   if (shell != "") {
+      switch (pid=fork()) {
+         case 0:
+            res=execl(shell, shell, "-l", "-c", buf, NULL);
+            break;
+         case -1:
+            return;
+         default:
+    	    wait(&status);
+	    break;
+      }
+   }
+
+   /* If that didn't work try to run /bin/sh in login mode */
+   if (WEXITSTATUS(status)==2 || res == -1 || shell == "") {
+      switch(pid=fork()) {
+         case 0: 
+   	    execl("/bin/sh", "/bin/sh", "-l", "-c", buf, NULL);
+	    break;
+         case -1:
+	    return;
+	 default: 
+	    wait(&status);
+	    break;
+      }
+   }
+
+
+   /* If /bin/sh isn't a login shell run /bin/sh without loading the profile
+    * Also log a warning because this will probably not behave correctly */
+   if (WEXITSTATUS(status)==2) { 
+      syslog(LOG_NOTICE, "Neither '%s' or '/bin/sh' are working login shells for user '%s'. Your session may not function properly. ",shell,user);
+      switch(pid=fork()) {
+ 	 case 0:
+	    execl("/bin/sh", "/bin/sh", "-c", buf, NULL);
+	 break;
+       case -1:
+	  return;
+       default:
+	  wait(&status);
+	  break;
+      }
+   }
+
+   /* Damn, that didn't work either.
+    * Bye! We call it quits and log an error */
+   if (WEXITSTATUS(status)==2) {
+      syslog(LOG_CRIT, "Entrance could not find a working shell to start the session for user: \"%s\".",user);
+   }
+}
Index: src/daemon/entranced_display.c
===================================================================
RCS file: /var/cvs/e/e17/apps/entrance/src/daemon/entranced_display.c,v
retrieving revision 1.2
diff -u -r1.2 entranced_display.c
--- src/daemon/entranced_display.c	1 Aug 2006 05:30:58 -0000	1.2
+++ src/daemon/entranced_display.c	30 Aug 2006 08:44:28 -0000
@@ -124,6 +124,11 @@
    double start_time;
    char x_cmd[PATH_MAX];
 
+   int i;
+   char *x_cmd_argv[32]; 
+
+   for (i=0;i<32;i++) x_cmd_argv[i]=NULL;
+
    /* Ecore_Exe *x_exe; */
    pid_t xpid;
 
@@ -160,9 +165,15 @@
         _entrance_x_sa.sa_flags = 0;
         sigemptyset(&_entrance_x_sa.sa_mask);
         sigaction(SIGUSR1, &_entrance_x_sa, NULL);
-      /* FIXME: need to parse command and NOT go thru /bin/sh!!!! */
-      /* why? some /bin/sh's wont pass on this SIGUSR1 thing... */
-        execl("/bin/sh", "/bin/sh", "-c", x_cmd, NULL);
+
+	x_cmd_argv[0]=strtok(x_cmd," ");
+	i=1;
+
+	while ((x_cmd_argv[i]=strtok(NULL," "))!=NULL) {
+	   i++;
+	}
+
+        execvp(x_cmd_argv[0], x_cmd_argv);
         syslog(LOG_WARNING, "Could not execute X server.");
         exit(1);
      default: