> +
> + /*
> +  *----------------------------------------------------------------------
> +  * Ns_SetPrivileges --
> +  *
> +  *      Set the effective user ID/group ID of the current process, uid as 
> -1 or
> +  *      gid as -1 will be ignored
> +  *
> +  * Results:
> +  *      -1 on error


Should be NS_OK / NS_ERROR.


> +  * Side effects:
> +  *      Process privileges will be different after success


This is not a side effect, it's the intended effect.


> + int
> + Ns_SetPrivileges(int uid, int gid)
> + {


This is really two calls in one. It should be something more like:

  int Ns_SetUser(CONST char *user);
  int Ns_SetGroup(CONST char *group);


Then Ns_GetPrivileges() can be deleted and rolled into the above.

For example, this:


  nsd/nsmain.c:

  if (Ns_SetPrivileges(-1, gid) == -1) {
      Ns_Fatal("nsmain: failed to switch to group %d", gid);
  }
  NsForkBinder();

  if (Ns_SetPrivileges(uid, -1) == -1) {
      Ns_Fatal("nsmain: failed to switch to user %d", uid);
  }


Can be turned into this:


  if (Ns_SetGroup(group) != NS_OK) {
      Ns_Fatal("nsmain: failed to switch to group %s", group);
  }
  NsForkBinder();

  if (Ns_SetUser(user) != NS_OK) {
      Ns_Fatal("nsmain: failed to switch to user %s", user);
  }


And this:


> RCS file: /cvsroot/naviserver/naviserver/nsproxy/nsproxylib.c,v
>
> --- 459,465 ----
>      }
>
> !     if (Ns_GetPrivileges(user, group, &uid, &gid) == -1 ||
> !         Ns_SetPrivileges(uid, gid) == -1) {
> !         Ns_Fatal("nsproxy: unable to switch to user '%s', group '%s'", 
> user, group);
>      }


Can be turned into this:


  if (Ns_SetUser(user) != NS_OK
          || Ns_SetGroup(group) != NS_OK) {
      Ns_Fatal("nsproxy: unable to switch to user '%s', group '%s'",
user, group);
  }

(Always put the || or && at the beginning of the continuation line.)


> + /*
> +  *----------------------------------------------------------------------
> +  *
> +  * NsTclSetPrivilegesObjCmd --
> +  *
> +  *      Implements ns_setprivileges as ObjCommand.


Should be ns_setuser and ns_setgroup.


> +  *
> +  * Results:
> +  *      Tcl result, 1 if sucessful, -1 on error


Should throw a Tcl error, not return -1.


> +  * Side effects:
> +  *      Error message will be output in the log file, not returned as Tcl 
> result


Use Tcl_PosixError().


>       NsTclUrlDecodeObjCmd,
>       NsTclUrlEncodeObjCmd,
>       NsTclVarObjCmd,
>       NsTclWriteContentObjCmd,
>       NsTclWriteFpObjCmd,
>       NsTclWriteObjCmd,
>       NsTclWriterObjCmd,
> -     NsTclFileStatObjCmd;
> +     NsTclFileStatObjCmd,
> +     NsTclSetPrivilegesObjCmd;


'S' comes after 'R' and before 'T'...


> Log Message:
>        * doc/src/ns_setpriveleges.man: Added documentation file


Checked, reasonably complete man pages go in doc/src/mann/*.


> + --- NEW FILE: ns_setpriveleges.man ---
> + [manpage_begin ns_setpriveleges n 4.99]


(Misspelled privilege)

Don't hard-code the version number. Do this:


  [include version_include]
  [manpage_begin ns_setprivileges n [vset version]]


> + [see_also nsd]
> + [keywords ns_time ns_conn]


...?!


> Index: tclmisc.c
> ===================================================================
> RCS file: /cvsroot/naviserver/naviserver/nsd/tclmisc.c,v
> retrieving revision 1.23
> retrieving revision 1.24
> diff -C2 -d -r1.23 -r1.24
> *** tclmisc.c   14 Aug 2007 08:49:10 -0000      1.23
> --- tclmisc.c   20 Oct 2007 17:20:32 -0000      1.24
> ***************
> *** 1071,1074 ****
> --- 1071,1084 ----
>   *
>   * $Log$
> +  * Revision 1.24  2007/10/20 17:20:32  seryakov
> +  *     * include/ns.h:
> +  *     * nsd/unix.c: Added 2 new public API functions Ns_SetPriveleges and 
> Ns_GetPriveleges
> +  *       which are resolve and assign user uid/gid
> +  *
> +  *     * nsproxy/nsproxylib.c:
> +  *     * nsd/nsmain.c: Switched to use new function that assign user real 
> uid/gid
> +  *
> +  *     * nsd/tclmisc.c: New Tcl command ns_setpriveleges that sets real uid 
> and gid
> +  *


We don't embed change logs in source files.


Also, *please* don't change white space in the same commit as other substantial
changes as it makes the update notifications very difficult to read.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
naviserver-devel mailing list
naviserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/naviserver-devel

Reply via email to