Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should we implement standard free semantics for deallocation functions? #345

Closed
Oppen opened this issue Jan 18, 2022 · 17 comments · Fixed by #643
Closed

Should we implement standard free semantics for deallocation functions? #345

Oppen opened this issue Jan 18, 2022 · 17 comments · Fixed by #643

Comments

@Oppen
Copy link
Collaborator

Oppen commented Jan 18, 2022

The standard library specifies free to act as a nop when its argument is a NULL pointer. This gives us programmers a big convenience for error handling and similar situations where we don't need to add so many explicit branches, but rather count on free doing the right thing for us. Point being, my proposal is not exactly something new.
Tackling #182 in particular would create (or, rather, acknowledge) many new error paths that will require these free analogs to be called, and the current implementations also mean explicitly checking for NULL each time on each call place. This can become tedious, error prone and a cognitive load rapidly.
It can also help brevity on existing code.

Counterarguments would probably go around the extra branching: sometimes we simply know for certain our pointer won't be NULL. IMO this case shouldn't matter that much for performance for the following reasons:

  1. Where operation count matters the most is in tight loops, and it's folklore already that you shouldn't allocate too much if you'll be discarding the buffer there. If you allocate it should be because you do need to keep the value afterwards, so how fast the free function is is not the central concern.
  2. Particularly error paths, and specifically those related to memory allocation, should be considered exceptional, and thus have low priority when it comes to performance. You'd rather optimize for the more common case, specially if your trade off is simply an extra branch.

Thoughts?

@lemire
Copy link
Member

lemire commented Jan 19, 2022

Yes. It is entirely fine to check for null when deallocating. But your rationale is excellent.

@mux
Copy link

mux commented Jul 26, 2024

The main argument against free() allowing NULL is not the extra branching (at least within the circle of people I interact with), but that it can easily hide bugs where some pointer being NULL is actually a symptom of a problem. A lot of people - not just me - disliked this C99 feature for this reason. I agree that the extra branching is not much of an issue because the performance hit is most likely going to be dwarfed by the cost of the deallocation itself. That being said, I tend to be very careful when making those judgement calls, because in my experience, 95% of the time you think something will never matter, somebody comes up with an example where it does.

@lemire
Copy link
Member

lemire commented Jul 26, 2024

@mux If the cost of the branch becomes significant, then I submit to you that we are doing too many allocations.

@Oppen
Copy link
Collaborator Author

Oppen commented Jul 27, 2024

but that it can easily hide bugs where some pointer being NULL is actually a symptom of a problem

Even if that's the case, calling free would be the wrong way to check. Even if passing NULL would be illegal, free is void, meaning the only other options are aborting or UB, none of which is desirable. If NULL is a symptom, the caller should check for it. The exact same way, calling our cleanup functions is the wrong way to check if the passed object is NULL.

A lot of people - not just me - disliked this C99 feature for this reason.

To the best of my knowledge and googling abilities, this feature is there since the first standard, and it's not a new C99 feature.

@mux
Copy link

mux commented Jul 27, 2024

but that it can easily hide bugs where some pointer being NULL is actually a symptom of a problem

Even if that's the case, calling free would be the wrong way to check. Even if passing NULL would be illegal, free is void, meaning the only other options are aborting or UB, none of which is desirable. If NULL is a symptom, the caller should check for it. The exact same way, calling our cleanup functions is the wrong way to check if the passed object is NULL.

Of course the caller should check for it, that's kind of a circular argument. We are talking about a situation in which there is a bug to begin with. The point is that free(NULL) crashing ensures that the developer will know about the situation if it does happen, and can correct the code, instead of it doing nothing and having potentially worse consequences later on. Fail early / fail fast.

A lot of people - not just me - disliked this C99 feature for this reason.

To the best of my knowledge and googling abilities, this feature is there since the first standard, and it's not a new C99 feature.

I am pretty sure it was first defined that free(NULL) should be safe to call in C99, at the same time than it was defined that malloc(0) should return a pointer that can then be passed to free(). I was a FreeBSD developer back then, and this generated very lively discussions as you can imagine.

@mux
Copy link

mux commented Jul 27, 2024

@mux If the cost of the branch becomes significant, then I submit to you that we are doing too many allocations.

My only point is that one should be careful before assuming that something isn't going to matter for everybody, as there is very often a possibility for this judgment to end up being wrong later on. I did say that I agreed it is fairly unlikely to be a problem.

In this specific case, it's easy to say someone is doing too many allocations - but you cannot always avoid doing that. Some people work within very specific environments, or have to work with existing code outside of their control that may be wrong in many ways. I don't think it takes a huge imagination to come up with a scenario where this could indeed be an issue, but I don't think there is a point in coming up with a contrived example here, this is more about a philosophy; being aware that something that might seem completely unrealistic to you might in fact happen to somebody someday.

@Dr-Emann
Copy link
Member

Dr-Emann commented Jul 27, 2024

https://web.archive.org/web/20200909074736if_/https://www.pdf-archive.com/2014/10/02/ansi-iso-9899-1990-1/ansi-iso-9899-1990-1.pdf

C89/90:

7.10.3.2 The free function

Synopsis

#include <stdlib.h>
void free(void *ptr);

Description

The free function causes the space pointed to by ptr to be deallocated. that is, made available for further allocation.

If ptr is a null pointer. no action occurs.

Otherwise, if the argument does not match a pointer earlier returned by the calloc. malloc, or realloc function, or if the space has been deallocated by a call to free or realloc, the behavior is
undefined.

(emphasis added)

@lemire
Copy link
Member

lemire commented Jul 27, 2024

StackOverflow might be wrong, but here is the top voted answer:

image

@lemire
Copy link
Member

lemire commented Jul 27, 2024

I have created a PR which, I think, would help solve this issue, please review
#643

@mux
Copy link

mux commented Jul 27, 2024

https://web.archive.org/web/20200909074736if_/https://www.pdf-archive.com/2014/10/02/ansi-iso-9899-1990-1/ansi-iso-9899-1990-1.pdf

C89/90:

7.10.3.2 The free function

Synopsis

#include <stdlib.h>
void free(void *ptr);

Description

The free function causes the space pointed to by ptr to be deallocated. that is, made available for further allocation.
If ptr is a null pointer. no action occurs.
Otherwise, if the argument does not match a pointer earlier returned by the calloc. malloc, or realloc function, or if the space has been deallocated by a call to free or realloc, the behavior is
undefined.

(emphasis added)

I stand corrected, those discussions I am remembering must have stemmed from something else than the introduction of C99. I stand by my preferences regarding this however, and the reasoning behind why I believe it is better for free() not to check for NULL.

@mux
Copy link

mux commented Jul 27, 2024

StackOverflow might be wrong, but here is the top voted answer:

image

That doesn't really make any sense since this behavior has nothing to do with the compiler and is about the standard C library.

@lemire
Copy link
Member

lemire commented Jul 27, 2024

That doesn't really make any sense since this behavior has nothing to do with the compiler and is about the standard C library.

I think it is customary to equate the two in non-formal settings. If I refer to GCC 14, for example, I refer to both the compiler and the libraries that come with it.

@mux
Copy link

mux commented Jul 27, 2024

That doesn't really make any sense since this behavior has nothing to do with the compiler and is about the standard C library.

I think it is customary to equate the two in non-formal settings. If I refer to GCC 14, for example, I refer to both the compiler and the libraries that come with it.

If it's customary then I guess it is a common mistake. The compiler and the standard library are simply two different pieces of software. In Linux systems, glibc is the libc implementation and it has a specific memory allocator implementation (ptmalloc these days, used to be dlmalloc IIRC) and FreeBSD, for instance, has its own libc implementation that uses the jemalloc memory allocator (and used to use phkmalloc a long time ago). I don't know if GCC comes with some malloc implementation, that is not impossible it would have one for very specific circumstances, but it isn't relevant anyways as this is not what would be used when you write a program that uses malloc.

@lemire
Copy link
Member

lemire commented Jul 27, 2024

I think that we understand the StackOverflow answer. From the very beginnng, free(NULL) had no effect in C, even though some implementations of the C standard might have differed.

@Oppen
Copy link
Collaborator Author

Oppen commented Jul 29, 2024

I stand corrected, those discussions I am remembering must have stemmed from something else than the introduction of C99. I stand by my preferences regarding this however, and the reasoning behind why I believe it is better for free() not to check for NULL.

It may very well be the case that you're talking pre-standard, if you worked on it at the time. C is much older than the standardization efforts. I have no idea what K&R C did, for example. I wasn't around (not even born) when C got standardized and only went as far back as the first standard. There's also a chance that ANSI and ISO differ in that.
In any case, it is well defined for any modern (2000+) compiler, and I think it makes sense to follow suit for the sake of minimizing surprises.

Re: assumptions: at some point you have to make some. Nothing stops us from reacting if and when the situation arises that our assumption is wrong, and then we fix it or work in a workaround. It doesn't make sense IMO to make all users pay proactively by making the interface unforgiving when the standard library is already much friendly, just in case some specific use case can't avoid allocating so much that the branch becomes a problem.
It's hard to be less forgiving than the C library, but let's not go out of our ways trying to.

@Oppen
Copy link
Collaborator Author

Oppen commented Jul 29, 2024

Lastly, regarding failing early, while I can agree with that philosophy, NULL dereference already ends up in a crash in most platforms (technically UB, but it's also what free(NULL) would probably be if not accepted as legal). free is also the last thing you do to a value. If you get to that point, I would guess you did all the harm you could, so crashing there doesn't really help.

@lemire
Copy link
Member

lemire commented Jul 29, 2024

@Oppen Can you review #643 ?

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

Successfully merging a pull request may close this issue.

4 participants