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 712136 - 'O_CLOEXEC' undeclared (first use in this function)
'O_CLOEXEC' undeclared (first use in this function)
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: build
2.39.x
Other Mac OS
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2013-11-12 08:17 UTC by Ryan Schmidt
Modified: 2013-11-23 16:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
seems easy enough? (735 bytes, patch)
2013-11-12 21:09 UTC, A. Walton
reviewed Details | Review
update with fd_cloexec (963 bytes, patch)
2013-11-12 21:30 UTC, A. Walton
accepted-commit_now Details | Review
preprocessor gotcha and the bug link added. (1.04 KB, patch)
2013-11-12 21:45 UTC, A. Walton
none Details | Review

Description Ryan Schmidt 2013-11-12 08:17:37 UTC
Following up on bug 710962, glib 2.39.1 doesn't build on OS X 10.6 Snow Leopard, with this error:

gsubprocess.c: In function 'unix_open_file':
gsubprocess.c:344: error: 'O_CLOEXEC' undeclared (first use in this function)
gsubprocess.c:344: error: (Each undeclared identifier is reported only once
gsubprocess.c:344: error: for each function it appears in.)

Here is a full log: https://build.macports.org/builders/buildports-snowleopard-x86_64/builds/22189/steps/compile/logs/stdio

It builds fine on OS X 10.7 through 10.9 inclusive.
Comment 1 A. Walton 2013-11-12 21:09:44 UTC
Created attachment 259696 [details] [review]
seems easy enough?

2c patch for the older BSDs.
Comment 2 Colin Walters 2013-11-12 21:13:00 UTC
Review of attachment 259696 [details] [review]:

For these systems, we should change unix_open_file() to set cloexec manually.  Something like:

#ifndef O_CLOEXEC
  fcntl (fd, F_SETFD, FD_CLOEXEC);
#endif
Comment 3 Colin Walters 2013-11-12 21:20:25 UTC
Although I could be convinced to not care about this, given that e.g. glocalfile.c doesn't attempt to make use of O_CLOEXEC.

But the lack of this means that if the current process includes (broken) software such as NSPR/Firefox that doesn't close fds before exec(), these fds will be leaked.

See
https://bugzilla.gnome.org/show_bug.cgi?id=672102#c130
https://bugzilla.gnome.org/show_bug.cgi?id=672102#c135
Comment 4 A. Walton 2013-11-12 21:30:21 UTC
Created attachment 259697 [details] [review]
update with fd_cloexec

(In reply to comment #2)
> Review of attachment 259696 [details] [review]:
> 
> For these systems, we should change unix_open_file() to set cloexec manually. 
> Something like:
> 
> #ifndef O_CLOEXEC
>   fcntl (fd, F_SETFD, FD_CLOEXEC);
> #endif

Oh, I was for some reason forgetful that FD_CLOEXEC existed; I knew O_CLOEXEC was a Linux-specific thing and for some reason thought the whole close-on-exec feature was Linux specific.

So something like this? (Presumably we don't need to GError report if this fails?)
Comment 5 Colin Walters 2013-11-12 21:35:02 UTC
Review of attachment 259697 [details] [review]:

Looks good to me.  Please add a link to this bug at the end of the commit message though, so people can find this discussion later; see https://live.gnome.org/GnomeLove/SubmittingPatches

Thanks!
Comment 6 A. Walton 2013-11-12 21:45:59 UTC
Created attachment 259700 [details] [review]
preprocessor gotcha and the bug link added.

Err, the last patch tests if it's not defined and then defines it... so when the preprocessor hits it later...

This patch fixes that and adds the missing bug link to the git commit message.

It'll have to wait until I'm at my home machine to be committed.