missing mkstemps function in stdlib.h

[kallisti5]

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.

​http://man7.org/linux/man-pages/man3/mkstemps.3.html

[korli]

See ​https://github.com/haiku/haiku/blob/master/src/system/libroot/posix/stdlib/mktemp.c#L56 ATM disabled

[kallisti5]

oh. that's weird. I don't see any history of it getting disabled. I'll test and see if it can be enabled.

[kallisti5]

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.

[pulkomandy]

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).

[kallisti5]

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.

[korli]

headers/compatibility/bsd/stdlib.h should be patched instead of headers/posix/stdlib.h

[kallisti5]

patch updated @ gerrit

[kallisti5]

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

[korli]

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.

[pulkomandy]

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. }}}

[kallisti5]

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)

[korli]

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).

[pulkomandy]

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.

[kallisti5]

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

[kallisti5]

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.

[mmu_man]

Is this still open?

Btw, NBDkit would need mkostemp() which has an additional flags argument, to pass O_CLOEXEC.

[mmu_man]

For mkostemp() see #15219.

[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.

[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))

[ahwayakchih]

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.

[pulkomandy]

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?

[ahwayakchih]

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.

[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

[ahwayakchih]

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 :).