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.

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

Attachments (3)

mkstemps.png (112.4 KB ) - added by kallisti5 7 years ago.
mkstemps_bsd.png (45.8 KB ) - added by kallisti5 7 years ago.
libbsd.png (81.5 KB ) - added by kallisti5 7 years ago.

Download all attachments as: .zip

Change History (28)

comment:2 by kallisti5, 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 kallisti5, 7 years ago

Owner: changed from nobody to kallisti5
Status: newassigned

comment:4 by kallisti5, 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 pulkomandy, 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 kallisti5, 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 kallisti5, 7 years ago

Attachment: mkstemps.png added

comment:7 by korli, 7 years ago

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

comment:8 by kallisti5, 7 years ago

patch updated @ gerrit

by kallisti5, 7 years ago

Attachment: mkstemps_bsd.png added

comment:9 by kallisti5, 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 korli, 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.

comment:11 by pulkomandy, 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 kallisti5, 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)

in reply to:  11 comment:13 by korli, 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 pulkomandy, 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 kallisti5, 7 years ago

Attachment: libbsd.png added

comment:15 by kallisti5, 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 kallisti5, 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 mmu_man, 5 years ago

Is this still open?

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

Last edited 5 years ago by mmu_man (previous) (diff)

comment:18 by mmu_man, 5 years ago

For mkostemp() see #15219.

comment:19 by ahwayakchih, 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.

in reply to:  19 ; comment:20 by korli, 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))

in reply to:  20 comment:21 by ahwayakchih, 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.

comment:22 by pulkomandy, 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?

in reply to:  22 ; comment:23 by ahwayakchih, 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.

in reply to:  23 ; comment:24 by korli, 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

in reply to:  24 comment:25 by ahwayakchih, 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 :).

Note: See TracTickets for help on using tickets.