[libc-commits] [PATCH] D138607: [libc] Move strdup implementation to a new header

Siva Chandra via Phabricator via libc-commits libc-commits at lists.llvm.org
Mon Nov 28 00:17:17 PST 2022


sivachandra added a comment.

I would think that GPU is one example of a platform where `malloc` isn't (yet) available. So, the mechanical aspects of this patch LGTM. I think we should do a few things more:

1. At the least, add comments warning developers that allocators should not be called from `string_utils.h`. Likewise, add comments explaining what exactly is `string_utils_malloc.h` (or whatever it will be called) for.
2. Does not have to be done in this patch, but would be nice if a developer mistake/mixup can be caught during build + test cycle. Worst case, the GPU builder will catch it I believe, but would be nice to be able to do it during a normal host build + test development cycle.



================
Comment at: libc/src/string/string_utils_malloc.h:1
+//===-- String utils malloc -------------------------------------*- C++ -*-===//
+//
----------------
`malloc` is one among a bunch of standard allocator functions. So, may be call this `allocating_string_utils.h`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138607



More information about the libc-commits mailing list