Skip to content

Conversation

@notcancername
Copy link

I've also added some inline tests, not quite sure how to run those eventually.

Comment on lines 1 to 18
#ifndef _PRIVATE_GETOPT_H
#define _PRIVATE_GETOPT_H

struct z_getopt_r_context {
char *optarg;
int opterr;
int optind;
int optopt;
int optcur;
};

int z_getopt_r(struct z_getopt_r_context *context, int argc, char *const argv[], const char *optstring);

extern char *optarg;
extern int opterr, optind, optopt;
int getopt(int, char * const [], const char *);
int getopt(int argc, char *const argv[], const char *optstring);

#endif /* _PRIVATE_GETOPT_H */
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these seems like private implementation details, I don't think we want or need any of these changes in the public libc headers

Copy link
Author

@notcancername notcancername Apr 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optcur is an implementation detail, while optarg, opterr, optind, optopt are used as specified. if you have a suggestion to hide this implementation detail, feel free to tell me :)

parameter names are useful for auto-completion and the like.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parameter names seem fine, why are we exposing the z_getopt_r_context and z_getopt_r symbols? Are these part of libc? If not, why are they in the libc headers? Why are we exporting them?

src/posix.zig Outdated
optcur: c_int = 0,
};

export fn z_getopt_r(context: *GetoptContext, argc: c_int, argv: [*]const ?[*:0]const u8, optstring: [*:0]const u8) callconv(.C) c_int {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this being exported?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to make a reentrant version available to the user.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is z_getopt_r a part of libc?

Copy link
Author

@notcancername notcancername Apr 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it should be exposed to the user as a normal part of libc. perhaps omitting the z_ prefix identifying it as a ziglibc extension would be good, but it's not my choice to make

Copy link
Owner

@marler8997 marler8997 Apr 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it should be exposed to the user as a normal part of libc

What I'm asking is, which standard libc is this function apart of? The cstd, glibc, musl, msvc. The purpose of ziglibc is to provide an alternative implementation for existing libc libraries.

Copy link
Author

@notcancername notcancername Apr 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not a part of any other libc, just a reentrant version of the posix function. if providing reentrant versions of functions is out of scope, it shouldn't be exposed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants