Opened 7 years ago
Last modified 4 years ago
#13996 assigned enhancement
missing mkstemps function in stdlib.h
Reported by: | kallisti5 | Owned by: | kallisti5 |
---|---|---|---|
Priority: | normal | Milestone: | Unscheduled |
Component: | System | Version: | R1/Development |
Keywords: | Cc: | ||
Blocked By: | Blocking: | ||
Platform: | All |
Description
Do we want to implement mkstemps ? It's more of a linux function. While I see mkstemp in stdlib.h, I don't see mkstemps.
Attachments (3)
Change History (28)
comment:1 by , 7 years ago
comment:2 by , 7 years ago
oh. that's weird. I don't see any history of it getting disabled. I'll test and see if it can be enabled.
comment:3 by , 7 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:4 by , 7 years ago
Hm.. per that article:
- "mkstemps(): unstandardized, but appears on several other systems."
For compatibility purposes, do we want to "just enable it" in stdlib (even though not in POSIX)? It'll make ports easier with fewer modifications.
comment:5 by , 7 years ago
We could put it under _BSD_SOURCE or some other similar guard (and finally implement a features.h so that people don't have to manually #define _BSD_SOURCE - but devs lazyness on that seems very high for some reason, they prefer patching hundreds of buildsystems to writing one header file apparently).
comment:6 by , 7 years ago
I pushed a review here for the "basic" version just copying linux + bsd.
https://review.haiku-os.org/#/c/97/
http://nixdoc.net/man-pages/FreeBSD/man3/mkstemps.3.html
So both linux and bsd just include it in stdlib :-|
Tested functional. see screenshot.
by , 7 years ago
Attachment: | mkstemps.png added |
---|
comment:7 by , 7 years ago
headers/compatibility/bsd/stdlib.h should be patched instead of headers/posix/stdlib.h
by , 7 years ago
Attachment: | mkstemps_bsd.png added |
---|
comment:9 by , 7 years ago
I do hate the _BSD_SOURCE requirements (screenshot) for something:
- That is defined by default on Linux
- That is defined by default on BSDs
That means we're left to porting + patching a lot of sources with "special Haiku things"
If it's available under Linux + BSD + MacOS, people are going to use it without a second thought even if it isn't POSIX
comment:10 by , 7 years ago
Well to be honest this function can stay hidden by default: We didn't need it until now, I don't see any patches for this function at HaikuPorts ( https://github.com/haikuports/haikuports/search?utf8=%E2%9C%93&q=mkstemps&type= ), so the projects requiring it might need a patch, that's fine.
And one more time it's not POSIX.
follow-up: 13 comment:11 by , 7 years ago
Once again...
The _BSD_SOURCE define is supposed to be set by default. As I mentioned above (and countless time already), we need to do this just like Linux and BSD and have a "features.h" which sets the define. I can't understand why people insist on the "let's patch everything we port" way instead, when it would be so easy to add that single header file with a few #ifdef/#define in it.
From the very Linux manpage, you can see that they also guard the definitions with some defines:
Feature Test Macro Requirements for glibc (see feature_test_macros(7)): mkstemp(): _XOPEN_SOURCE >= 500 || /* Since glibc 2.12: */ _POSIX_C_SOURCE >= 200809L || /* Glibc versions <= 2.19: */ _SVID_SOURCE || _BSD_SOURCE mkostemp(): _GNU_SOURCE mkstemps(): /* Glibc since 2.19: */ _DEFAULT_SOURCE || /* Glibc versions <= 2.19: */ _SVID_SOURCE || _BSD_SOURCE mkostemps(): _GNU_SOURCE
Further reading: http://man7.org/linux/man-pages/man7/feature_test_macros.7.html
The idea is basically to enable this by default, unless someone requested strict C standard (--std=c99 or the like) from the compiler command line. Not defining the extra functions in that case is actually a requirement, because the C language has an exact list of what each header must provide (nothing more, nothing less). This is why there has to be a way of disabling the extra functions. If on the other hand an "extended" standard is used (like --std=gnu89/gnu11, which are the default), then we do not need to strictly comply to C anymore and can make the extensions visible.
And from gcc docs (https://gcc.gnu.org/onlinedocs/gcc-4.0.4/gcc/C-Dialect-Options.html):
The macro __STRICT_ANSI__ is predefined when the -ansi option is used. Some header files may notice this macro and refrain from declaring certain functions or defining certain macros that the ISO standard doesn't call for; this is to avoid interfering with any programs that might use these names for other things.
So basically our features.h should look something like:
#ifndef __STRICT_ANSI__ #define _BSD_SOURCE #endif
We then #include this in all headers needing _BSD_SOURCE, and developers never ever need to #define anything anymore. }}}
comment:12 by , 7 years ago
PulkoMandy, we already do have a few situations where _BSD_SOURCE is defined:
https://git.haiku-os.org/haiku/tree/src/system/libroot/posix/glibc/include/features.h#n116
I'm guessing a check like yours would go under the "favor bsd" part since "not favoring BSD over POSIX" would make sense unless _BSD_SOURCE is defined.
There is a defined _BSD_SOURCE at: https://git.haiku-os.org/haiku/tree/src/system/libroot/posix/glibc/include/features.h#n150
It never seems to trigger though (as shown in my example, you have to -D_BSD_SOURCE)
comment:13 by , 7 years ago
Replying to PulkoMandy:
Once again...
Sure, if you'd like to propose a change, and not implement it yourself, be nice to submit an enhancement/bug ticket (or post a link if it exists).
comment:14 by , 7 years ago
Yes, that features.h from glibc looks mostly right. Then all we need to do is #include it everywhere in our other .h files where we have a #ifdef _BSD_SOURCE.
by , 7 years ago
Attachment: | libbsd.png added |
---|
comment:15 by , 7 years ago
that still doesn't solve the -lbsd requirement though of moving the function to src/libs/bsd/mkstemps.c (attached example)... unless we want to leave mkstemps in posix but expose it in bsd stdlib.h
comment:16 by , 7 years ago
huh.. waddlesplash just merged my review as-is. This issue isn't solved, don't take that commit as it being solved.
As-is, mkstemps is in libroot posix. Do we want it to just live there? I have the code locally moving it to libbsd.
comment:17 by , 5 years ago
Is this still open?
Btw, NBDkit would need mkostemp() which has an additional flags argument, to pass O_CLOEXEC.
follow-up: 20 comment:19 by , 4 years ago
Library for a next JPEG-XL format started to use mkstemps
in their benchmark code (https://gitlab.com/wg1/jpeg-xl/-/blob/c8ce59f8ef6393ba3018917791d04662cbf1172a/tools/benchmark/benchmark_utils.cc#L41). While benchmarks are not needed to use library, they are compiled by default and cause build to fail.
follow-up: 21 comment:20 by , 4 years ago
Replying to ahwayakchih:
Library for a next JPEG-XL format started to use
mkstemps
in their benchmark code (https://gitlab.com/wg1/jpeg-xl/-/blob/c8ce59f8ef6393ba3018917791d04662cbf1172a/tools/benchmark/benchmark_utils.cc#L41). While benchmarks are not needed to use library, they are compiled by default and cause build to fail.
mkstemps is available if
#if defined(_BSD_SOURCE) \ || (!defined(__STRICT_ANSI__) && !defined(_POSIX_C_SOURCE))
comment:21 by , 4 years ago
Replying to korli:
Replying to ahwayakchih:
Library for a next JPEG-XL format started to use
mkstemps
in their benchmark code (https://gitlab.com/wg1/jpeg-xl/-/blob/c8ce59f8ef6393ba3018917791d04662cbf1172a/tools/benchmark/benchmark_utils.cc#L41). While benchmarks are not needed to use library, they are compiled by default and cause build to fail.mkstemps is available if
#if defined(_BSD_SOURCE) \ || (!defined(__STRICT_ANSI__) && !defined(_POSIX_C_SOURCE))
Yes, it works, but only after i passed additional -D_BSD_SOURCE
through CMAKE_FLAGS.
I guess that's usable enough :), but maybe it would be good to add comment in bsd/stdlib.h to pass such define (so, after i search through headers, and find it there, i know what to add in CLI)?
Best would be to make GCC error hint about such possibility, but that would probably require some very specific list of cases, which would add a lot of work.
follow-up: 23 comment:22 by , 4 years ago
Well, the feature is enabled by default if you don't specify anything. It is hidden only if you put gcc in strict mode (disable GNU extensions) by using, for example, -std=c99 (instead of -std=gnu99 which leaves extensions enabled).
It's jpeg-xl fault for using a function not in the standard, and explicitly asking the compiler to follow the standard.
We can document it, sure. Should we add it to api.haiku-os.org under a "libbsd" section?
follow-up: 24 comment:23 by , 4 years ago
Replying to pulkomandy:
It's jpeg-xl fault for using a function not in the standard, and explicitly asking the compiler to follow the standard.
I'm not sure any "strict" mode is specified. I'll have to check that next time.
We can document it, sure. Should we add it to api.haiku-os.org under a "libbsd" section?
Yeah. Or maybe "Porting software from Linux/BSD" special section, and "libbsd" subsection there?
Although, -lbsd
was not needed in this case, as mkstemps
is in libroot
anyway. Which may be a bit confusing.
follow-up: 25 comment:24 by , 4 years ago
Replying to ahwayakchih:
It's jpeg-xl fault for using a function not in the standard, and explicitly asking the compiler to follow the standard.
I'm not sure any "strict" mode is specified. I'll have to check that next time.
see https://gitlab.com/wg1/jpeg-xl/-/blob/c8ce59f8ef6393ba3018917791d04662cbf1172a/CMakeLists.txt#L170
comment:25 by , 4 years ago
Replying to korli:
Replying to ahwayakchih:
It's jpeg-xl fault for using a function not in the standard, and explicitly asking the compiler to follow the standard.
I'm not sure any "strict" mode is specified. I'll have to check that next time.
see https://gitlab.com/wg1/jpeg-xl/-/blob/c8ce59f8ef6393ba3018917791d04662cbf1172a/CMakeLists.txt#L170
Thanks, i passed the info :).
See https://github.com/haiku/haiku/blob/master/src/system/libroot/posix/stdlib/mktemp.c#L56 ATM disabled