GNOME Bugzilla – Bug 623890
brasero-session.c uses mkdtemp(), which is not a standard function
Last modified: 2011-10-21 06:51:01 UTC
building brasero 2.30.2 on x86_64-sun-solaris2.10. I get a link failure when building libbrasero-burn because brasero-session uses mkdtemp, which is not a standardized symbol (yet, it looks like it's due in the next X/Open standard). I just hacked this into brasero-session.c to get around the problem: #if defined(__sun) char *mkdtemp(char * template) { if (!mktemp(template) || mkdir(template,0700)) return NULL; return template; } #endif Obviously that's not a good permanent fix. If I add a HAVE_MKDTEMP autoconf check and wrap that function with that, would it be sufficient?
I don't agree with adding HAVE_MKDTEMP conditional, solaris have any problem compiling brasero ?
I'm not certain I understand the question. Other than the fact that Solaris doesn't have mkdtemp so some type of replacement is needed, the rest of brasero compiles just fine on x86_64-sun-solaris2.10, and I'm using the Sun compiler suite, which is generally a little more picky than gcc. I haven't yet tested how well brasero works, but with the exception of the missing mkdtemp(), it compiles fine. So that I might understand, what's wrong with adding an autoconf check for mkdtemp() and providing a replacement function on platforms where mkdtemp is missing?
Note to improve your patch slightly. Building on Solaris is already checked with HAVE_USCSI_H which indicates that we are building against the Solaris SCSI interface; so you do not have to add another check at configure time. The other problem (inherent to the use of mktemp ()) is that in between mktemp () and mkdir () a malicious app could create a directory or open file and "steal" the location... Not that it seems a big risk for brasero (I admit =)) which is not a big target for malicious code provided their programmers ever heard of it.
Checking if you compile on solaris is not a good idea, since opensolaris does have mkdtemp in the standard library. Adding a HAVE_MKDTEMP conditional seems the clean way to solve this. I attach patches against brasero-2.30.2. They are made the way the problem is solved in gdm.
Created attachment 171688 [details] [review] Adding HAVE_MKDTEMP to configure.in
Created attachment 171689 [details] [review] Add mkdtemp fix to libbrasero-burn/Makefile.am
Created attachment 171690 [details] [review] Adding mkdtemp.h to libbrasero-burn/brasero-session.c
Created attachment 171691 [details] [review] mkdtemp implementation from gdm/gnu c library
Created attachment 171692 [details] [review] mkdtemp.h implementation from gdm/gnu c library
This should be fixed in glib, not locally in every application that needs it.
(In reply to comment #10) > This should be fixed in glib, not locally in every application that needs it. Sounds a better way but how should it be fixed in glib ? adding a new function providing mkdtemp ? Should we forward to glib maintainers ?
Created attachment 199572 [details] [review] use GLib g_mkdtemp() rather than mkdtemp() There is already g_mkdtemp(), which was added for GLib 2.26: http://developer.gnome.org/glib/unstable/glib-File-Utilities.html#g-mkdtemp The attached patch make Brasero use the GLib version, rather than a local implementation.
Comment on attachment 199572 [details] [review] use GLib g_mkdtemp() rather than mkdtemp() Pushed to master as commit 5f6524df57094b0fea5e3c55e5d79eef85b226a0.