joerg added a comment.
Why do we need to allocate memory in this case at all? I.e. why can't this just
be:
if (S.empty())
return StringRef("", 0);
...
Repository:
rL LLVM
https://reviews.llvm.org/D50966
___
cfe-commits mailing list
cfe-
This revision was automatically updated to reflect the committed changes.
Closed by commit rL340170: Fix an undefined behavior when storing an empty
StringRef. (authored by hokein, committed by ).
Repository:
rL LLVM
https://reviews.llvm.org/D50966
Files:
llvm/trunk/lib/Support/StringSaver.
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
lg
Comment at: lib/Support/StringSaver.cpp:14
StringRef StringSaver::save(StringRef S) {
+ if (S.empty())
hokein wrote:
> ioeric wrote:
> > Is it possibl
hokein added inline comments.
Comment at: lib/Support/StringSaver.cpp:14
StringRef StringSaver::save(StringRef S) {
+ if (S.empty())
ioeric wrote:
> Is it possible to reuse `StringRef::copy(Allocator &)` here?
Unfortunately, not, `StringRef::copy` will change
hokein updated this revision to Diff 161467.
hokein added a comment.
Correct the fix.
Repository:
rL LLVM
https://reviews.llvm.org/D50966
Files:
lib/Support/StringSaver.cpp
Index: lib/Support/StringSaver.cpp
===
--- lib/Supp
ioeric added inline comments.
Comment at: lib/Support/StringSaver.cpp:14
StringRef StringSaver::save(StringRef S) {
+ if (S.empty())
Is it possible to reuse `StringRef::copy(Allocator &)` here?
Repository:
rL LLVM
https://reviews.llvm.org/D50966
_
hokein created this revision.
hokein added a reviewer: ioeric.
Herald added a subscriber: llvm-commits.
Passing a nullptr to memcpy is UB.
Repository:
rL LLVM
https://reviews.llvm.org/D50966
Files:
lib/Support/StringSaver.cpp
Index: lib/Support/StringSaver.cpp
===
