[libc-commits] [PATCH] D139584: [libc] Add custom operator new to handle allocation failures gracefully.

[Tue Ly via Phabricator via libc-commits]

lntue added inline comments.


================
Comment at: libc/src/__support/CPP/new.h:54
+inline void *operator new(size_t size, __llvm_libc::AllocChecker *ac) noexcept {
+  return __llvm_libc::AllocChecker::alloc(size, *ac);
+}
----------------
What should happen if `ac == nullptr`?  Does second parameter has to be a pointer according to the standard, or it can be a reference instead?


================
Comment at: libc/src/string/allocating_string_utils.h:27
+  AllocChecker ac;
+  char *newstr = new (&ac) char[len];
+  if (!ac)
----------------
sivachandra wrote:
> sivachandra wrote:
> > tschuett wrote:
> > > sivachandra wrote:
> > > > sivachandra wrote:
> > > > > tschuett wrote:
> > > > > > lntue wrote:
> > > > > > > Does it mean that our `strdup` will always return aligned strings?
> > > > > > Would in this case a malloc returning a result or error type easier to understand and safer?
> > > > > > Does it mean that our `strdup` will always return aligned strings?
> > > > > 
> > > > > This is should call the non-aligning `operator new`. Why do you think it would always calls the aligning `operator new`?
> > > > > Would in this case a malloc returning a result or error type easier to understand and safer?
> > > > 
> > > > Couple of points:
> > > > 
> > > > 1. We want the libc source code to look as much as possible like a normal modern C++ library.
> > > > 2. Using `new`/`delete` for allocation/deallocation has the added benefit that compilers will synthesize constructor/destructor calls appropriately. It is not relevant here I would think, but uniformity helps improve readability and keeps coding guidlines/principles simple.
> > > `std::expected` will be modern C++.
> > Sure, `std::expected` is a very good choice in a few places. What I am referring to here is using `new`/`delete` vs a custom allocator function which wraps `malloc`/`free`.
> I have passed on your suggestion about `std::expected` here: https://reviews.llvm.org/D139576
> > Does it mean that our `strdup` will always return aligned strings?
> 
> This is should call the non-aligning `operator new`. Why do you think it would always calls the aligning `operator new`?

Never mind, I was looking at wrong signatures.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139584/new/

https://reviews.llvm.org/D139584