I'm agreeing with you, to be clear.

But buggy software might not crash; it might compare NULL == NULL and misbehave, instead.

So there's really no good option here, which is why C is a rancid choice of language for publicly-usable APIs. But we're stuck with it. So it goes.

I've been programming in C since 1978 and never until now heard of a function returning "*" on error.

This specific manifestation of "C is terrible for shared code APIs" is rare.

But issues caused by folks forgetting to check errno, miscomparing NULL to NULL when calling multiple APIs, dereferencing external-API-returned NULL due to missed error checking, mishandling inherited errno after successful action, or--the most common--mishandling negative integer return sentinels meant to indicate errors? Those are incredibly common.

This specific library call is quite poorly designed.

Standard C api calls return an integer, which is 0 on success or nonzero on error. Ignoring it is an intentional choice. `errno` is the hack that shouldn't really exist because it's trivially easy to forget to check that and also has issues with threading (patched by making it a thread local) and more recently, coroutines (which admit no general purpose patch because they're a userspace construct).

Another reason why this is terrible is that it's unclear what the returned pointer's lifetime is. Is it until the next call to crypt? If so, is it thread-local like errno? If it's passed on to the user, is it safe to call free() on it, or is there a crypt_free function that should be called instead?

But all of this, and I mean all of it pales in comparison to returning '*' on error.

> But all of this, and I mean all of it pales in comparison to returning '*' on error.

100% agree. This is an egregious failure. But C's error-handling story would be bad even without this, and the badness of C's error handling is what motivated the (bad) decision to return *.

It'd be great if we wrote systems software in a language that supported advanced features like...

checks notes

...returning multiple values.

Hell, even in VBA you had to at least spend the keystrokes to type "on error resume next".

It's just that every other string than "*" would have been a better choice, including even the empty string (which would have been the next terrible choice).

It's not a C thing, it's a Linux thing. The asterisk is relevant to how crypt() is used for /etc/shadow.

           If the password field contains some string that is not a valid result of crypt(3), for instance ! or *, the user will not
           be able to use a unix password to log in (but the user may log in the system by other means).

No, it's a C thing. They previously returned NULL for invalid passwd lookups, but because there's no way in C to force API users to decide what to do if an error occurred, that was causing program crashes due to NULL dereferences.

So they made the (very bad, but motivated by C's broken design/conventions) decision to return a magic string instead.

Oh, it was magic alright.

This sounds like a program problem not a library problem. Why did the library try to solve it?

Now imagine every other C API that can potentially return NULL did the same thing. Is that in any way sensible?

You can argue that C has broken design/conventions, but the musl decision was really really awkward.

> but because there's no way in C to force API users to decide what to do if an error occurrs,

0. There are tons of libc functions that return NULL on failure, it is conventional way in C to indicate failure.

1. There's no much to decide, just fail to login if crypt() fails with NULL.

2. errno is standard way in C to check error code of a failing function, but in case of login you hardly need this, just fail!

I totally agree that fixing this issue in musl was a pretty dubious solution. Don't fix what's not broken, fix broken application instead. Publish a CVE if you cannot fix it.

I looked through FreeBSD source code for how they use crypt(), it's checked for NULL everywhere!

Nothing you said is incorrect, but I believe they meant that there’s no way to force a C program to check the NULL correctly—the compiler will happily compile code that gets a pointer back from a function but doesn’t check for NULL before dereferencing it. In something like Rust the type system won’t let you touch the value until you’ve done something to deal with the error. Even if you just want to ignore it, you are forced to acknowledge that the function is fallible by the type system—it’s a compile error if you don’t.

Returning NULL on error is actually an excellent choice because most programs will crash if they try to use it without checking. Of course, modern programming languages have better constructs, but this was rare to nonexistent when C was started and if you think C is going away anytime soon you're delirious. musl's decision to change that to "*" is ununderstandable.

> musl's decision to change the return value to "*" is ununderstandable and likely intentionally evil.

Dude, they documented their rationale. It was dumb and wrong but totally relatable. Irresponsible users were getting crashes, and I’m sorry to tell you that for a lot of library maintainers

> Returning NULL on error is actually an excellent choice because most programs will crash if they try to use it without checking.

…is not something they want to write in responses to bug reports, even though it is quite true. It’s human nature to want to prevent your stuff causing problems for others, even if the problems are fully their fault. Most people cannot and will not be Steve “so don’t hold it like that” Jobs, for fear that their decision will be just as short-sighted as his was.

In this case, that was obviously the dead wrong call. But I’ve been in that seat, and “closed: user error” for the 100th time feels bad to write. Add to that a touch of misplaced cleverness, and you get “hey…what if we just returned an impossible string instead of an error?”

Evil requires motive or malice, and I’m having trouble coming up with those in this case. These patches weren’t concealed or miscategorized a la xz, the behavior was discussed in the open. They were just, in hindsight, really poor decisions made for ordinary reasons. Hanlon’s Razor and all that.

> “closed: user error” for the 100th time feels bad to write.

That's the job of a OSS library author.

> That’s why I’ve been harping on C being trash for shared-code API…

Agreed, but I can’t think of any other widespread ABIs that are better. Rust’s is not stable both C++ and Rust have concepts that don’t map into other languages well, so using them wholesale is a no go. So it would probably need to be a subset of those. Honestly just tagged unions would probably be enough. Maybe the solution is to get native tagged unions into C…

Fine, I can follow your reasoning.

There is the concept of "defense in depth": having each layer of code and configuration be sane and secure. Even if you program in Rust, you should be vigilant in that sense. This kind of code amounts to removing one or two layers.

C programs wouldn't accidentally compare NULL to NULL. They'd either check for null explicitly, or they'd pass the null to strcmp and crash.