[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