Remix.run Logo
cesaref 5 hours ago

Is it only me that would have expected curl_getenv() to have an assert that it's argument isn't NULL?

I know this doesn't stop runtime problems in release builds, but i'd have thought this sort of simple precondition check would help users find problems in their library useage.

It's not going to stop you passing a non-terminated string, or other such invalid input though, which is I guess more the point, that it's totally possible in C to produce good looking but actually invalid arguments that can't be spotted at runtime without UB (out of bounds access etc).

Edit: Actually thinking about this more, I guess the problem is that you are likely linking against a release library implementation, so it's not possible to add a precondition without introducing a runtime overhead, which is probably more likely what we are talking about with this case.

hathawsh 5 hours ago | parent | next [-]

If I were doing a code review, I would probably accept the code either with or without the assertion. The context of curl_getenv() makes it clear that null is not acceptable. If the author of curl_getenv() had evidence that callers are frequently breaking the contract by passing null, then perhaps the assertion would help shed some light on violators. Otherwise, I would expect everyone to play by the rules, making the assertion unnecessary.

vintagedave 20 minutes ago | parent | next [-]

That is exactly why you have a precondition or assertion.

If everyone expects specific behavior - ie it’s in the contract - you require that contract.

hathawsh 15 minutes ago | parent [-]

Yes, but null pointers are so pervasive in C code that we really can't afford to put assertions everywhere. It's often better to let the app crash on violations.

dwattttt 2 minutes ago | parent [-]

An assertion is an app crashing on a violation. The problem is when it's not guaranteed to crash, and instead does something very wrong.

favorited 3 hours ago | parent | prev [-]

It's also just a wrapper around getenv that provides consistent behavior across platforms, and passing a NULL name to the POSIX getenv function is UB.

gkfasdfasdf an hour ago | parent | prev [-]

In addition to an assert, a 'nonnull' attribute on compilers that support it seems like a good idea.