Opened 22 months ago

Last modified 4 months 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:
Has a Patch: no 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 22 months ago.
mkstemps_bsd.png (45.8 KB ) - added by kallisti5 22 months ago.
libbsd.png (81.5 KB ) - added by kallisti5 22 months ago.

Download all attachments as: .zip

Change History (21)

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

Owner: changed from nobody to kallisti5
Status: newassigned

comment:4 by kallisti5, 22 months 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, 22 months 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, 22 months 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, 22 months ago

Attachment: mkstemps.png added

comment:7 by korli, 22 months ago

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

comment:8 by kallisti5, 22 months ago

patch updated @ gerrit

by kallisti5, 22 months ago

Attachment: mkstemps_bsd.png added

comment:9 by kallisti5, 22 months 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, 22 months 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, 22 months 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, 22 months 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, 22 months 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, 22 months 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, 22 months ago

Attachment: libbsd.png added

comment:15 by kallisti5, 22 months 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, 22 months 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, 4 months ago

Is this still open?

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

Last edited 4 months ago by mmu_man (previous) (diff)

comment:18 by mmu_man, 4 months ago

For mkostemp() see #15219.

Note: See TracTickets for help on using tickets.