This revision was automatically updated to reflect the committed changes.
Closed by commit rL335612: Represent invalid UUIDs as UUIDs with length zero
(authored by labath, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D48479?vs=152699
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.
Patch is good. Feel free to remove the DataExtractor::DumpUUID() in a separate
NFC commit or just remove it in this patch
Comment at: source/Utility/DataExtractor.cpp:11
labath added inline comments.
Comment at: include/lldb/Utility/UUID.h:109
+
+ uint32_t m_num_uuid_bytes = 0; // Should be 0, 16 or 20
ValueType m_uuid;
clayborg wrote:
> Do we need this comment here? We currently take a 4 byte debug info CRC and
> call it a
clayborg added a comment.
Factory methods make things much clearer.
https://reviews.llvm.org/D48479
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
clayborg added a comment.
We should allow 4 and 8 byte UUIDs as pointed out by inlined comments. This
means we should probably modify the UUID dumper to handle those cases as well.
Comment at: include/lldb/Utility/UUID.h:109
+
+ uint32_t m_num_uuid_bytes = 0; // Should be 0,
labath updated this revision to Diff 152699.
labath added a comment.
- Use static factory functions instead of the extra argument (the best names I
could come up with is fromData and fromOptionalData).
https://reviews.llvm.org/D48479
Files:
include/lldb/Utility/UUID.h
source/API/SBModuleSp
labath added a comment.
In https://reviews.llvm.org/D48479#1141274, @clayborg wrote:
> Would love to remove the "accept_zeroes" argument everywhere. Too much
> matching happens in LLDB and we can't have multiple shared libraries claiming
> zeros as their UUID
A zero UUID is a problem only if
clayborg added a comment.
Would love to remove the "accept_zeroes" argument everywhere. Too much matching
happens in LLDB and we can't have multiple shared libraries claiming zeros as
their UUID
Comment at: include/lldb/Utility/UUID.h:54
+ void SetBytes(const void *uuid_byte
>
> I'm not sure if there is a suitable place for that function. This is
> needed in "ObjectFileMachO" and two dynamic loader plugins.
Then your factory idea may be the next best thing. While we're at it, maybe
we can remove the UUID::SetBytes() from the public interface, and make the
UUID an im
lemo added a comment.
> I'm not sure if there is a suitable place for that function. This is
> needed in "ObjectFileMachO" and two dynamic loader plugins.
Then your factory idea may be the next best thing. While we're at it, maybe
we can remove the UUID::SetBytes() from the public interface, an
labath added a comment.
In https://reviews.llvm.org/D48479#1141067, @lemo wrote:
> One solution might be to encapsulate the MachO convention in the MachO
>
> code: check in there (maybe through a helper function) if the UUID is
> "000...0" and map it to the empty UUID in that case. The UUID in
lemo added subscribers: clayborg, labath, sas.
lemo added a comment.
> However, during parsing you need to know the meaning of a "...0" UUID.
> In a MachO file (at least based on the comments in the code) this value is
> used to denote the fact that the object file has no UUID. For elf, a
>
>
> However, during parsing you need to know the meaning of a "...0" UUID.
> In a MachO file (at least based on the comments in the code) this value is
> used to denote the fact that the object file has no UUID. For elf, a
> "000..0" build-id is a perfectly valid identifier (and the lack of a
>
labath updated this revision to Diff 152530.
labath added a comment.
Delete copy constructor.
https://reviews.llvm.org/D48479
Files:
include/lldb/Utility/UUID.h
source/API/SBModuleSpec.cpp
source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
source/Plugins/DynamicLoa
labath added a comment.
In https://reviews.llvm.org/D48479#1140927, @lemo wrote:
> > The slight complication here is that
> > some clients (MachO) actually use the all-zero notation to mean "no UUID
> > has been set". To keep this use case working, I have introduced an
> > additional argument
lemo added a comment.
> The slight complication here is that
> some clients (MachO) actually use the all-zero notation to mean "no UUID
> has been set". To keep this use case working, I have introduced an
> additional argument to the UUID constructor, which specifies whether an
> all-zero vect
labath created this revision.
labath added reviewers: clayborg, sas, lemo, davide.
Herald added subscribers: arichardson, emaste.
Herald added a reviewer: espindola.
During the previous attempt to generalize the UUID class, it was
suggested that we represent invalid UUIDs as length zero (previousl
17 matches
Mail list logo
