Alan/Joerg:

Thanks, I updated the patch based on your comments.  Please review and
let me know if there are further issues.

Alan:

> +             g_unlink (GDM_SDTLOGIN_DIR);
> 
> This seems to result in calling the unlink(2) system call (at
> least in the upstream code at [1]).
> 
> As noted in the unlink(2) man page, unlink on a directory on
> Solaris UFS either fails (if not called by root) or  corrupts the
> filesystem, requiring fsck to fix (if called by root).
> 
> unlink on a directory on Solaris ZFS just fails for everyone.
> 
> You should always use remove() instead of unlink() if the argument
> could ever be a directory.   Perhaps glib's g_unlink() should be
> modified to just always call remove() instead, at least on Solaris.
> 
> [1] http://svn.gnome.org/viewvc/glib/trunk/glib/gstdio.c?view=markup#l528

Good catch.  I updated the patch so that it uses the GLib g_remove
function when removing directories.  I noticed a few other places
(in auth.c and misc.c) where directories were being removed via
g_unlink and also changed them to use the safer g_remove.  The GLib
g_remove function should also address Joerg's concerns about
portability.

Joerg:

 >  /* PRIO_MIN and PRIO_MAX are not defined on Solaris, but are -20 and 
20 */
 > -#if sun
 > +#if __sun
 >  #ifndef PRIO_MIN
 >  #define PRIO_MIN -20
 >  #endif
 >
 > If this is related to nice(2), this seems to be a missunderstanding....
 >
 > See NZERO in sys/param.h and man -s 2 nice in this case.

Also a good catch.  Note these is used with setpriority(3c) rather
than nice(2).  However, the values for min/max values are the same
for both according to the setpriority man page.  I also updated the
patch so that the priority values are now managed properly on Solaris.

Brian

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: gdm-08-createdt.diff
URL: 
<http://mail.opensolaris.org/pipermail/jds-review/attachments/20090106/243d245c/attachment.ksh>

Reply via email to