After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 357585 - Calls to set_cloexec inefficient on Solaris
Calls to set_cloexec inefficient on Solaris
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
unspecified
Other Solaris
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2006-09-25 10:02 UTC by padraig.obriain
Modified: 2006-12-15 08:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch to use fdwalk if available (1.41 KB, patch)
2006-09-25 12:43 UTC, padraig.obriain
none Details | Review
alternative patch (1.94 KB, patch)
2006-10-01 21:41 UTC, Matthias Clasen
none Details | Review
man page for fdwalk (4.08 KB, text/plain)
2006-10-02 07:49 UTC, padraig.obriain
  Details

Description padraig.obriain 2006-09-25 10:02:52 UTC
Traversing all possible fds and calling set_cloexec in gspawn.c is inefficient
on SOlaris.

The function fdwalk should be used.

I will supply a patch
Comment 1 padraig.obriain 2006-09-25 12:43:31 UTC
Created attachment 73366 [details] [review]
Proposed patch to use fdwalk if available
Comment 2 Matthias Clasen 2006-09-25 13:40:56 UTC
I think we don't want the #ifdef __sun here:

+#ifdef __sun
+#include <stdlib.h>
+#endif

I'd rather make that

#include <stdlib.h>  /* for fdwalk */
Comment 3 padraig.obriain 2006-09-25 13:51:53 UTC
Yes. Sorry about that.
Comment 4 Matthias Clasen 2006-10-01 06:17:16 UTC
Actually, I think it would be nicer to restructure our cloexec code to 
simple implement fdwalk if it is not available.
Comment 5 Matthias Clasen 2006-10-01 21:41:01 UTC
Created attachment 73782 [details] [review]
alternative patch

Does this look ok ?
Comment 6 padraig.obriain 2006-10-02 07:49:50 UTC
Created attachment 73829 [details]
man page for fdwalk

This work for me. You might want to consider making fdwalk return a value. I have attached fdwalk man page for reference.
Comment 7 padraig.obriain 2006-10-12 12:33:46 UTC
There was an error in my original patch. The function fdwalk should call return (0);
Comment 8 Matthias Clasen 2006-12-15 05:36:40 UTC
2006-12-15  Matthias Clasen  <mclasen@redhat.com>

	Fix #357585, Padraig O'Briain.
	
	* configure.in: Check for fdwalk.
	
	* glib/gspawn.c (do_exec): Use fdwalk() to close all
	file descriptors. 

	* glib/gspawn.c (fdwalk): Fallback implementation of
	fdwalk.
Comment 9 James Andrewartha 2006-12-15 08:02:49 UTC
This patch caused the build to fail: http://jhbuild.bxlug.be/builds/2006-12-15-0005/logs/glib/#build