<div dir="ltr"><div>Here's a snapshot of the old changes I had: <a href="https://reviews.llvm.org/D48381">https://reviews.llvm.org/D48381</a></div><div>(hopefully it helps a bit but caveat emptor: this is a quick merge from an old patch, so it's for illustrative purposes only)</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jun 20, 2018 at 10:26 AM, Pavel Labath <span dir="ltr"><<a href="mailto:labath@google.com" target="_blank">labath@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">From the looks of it, the patch stalled on the part whether we can<br>
consider all-zero UUIDs as valid or not. I've dug around the code a<br>
bit now, and I've found this comment in ObjectFileMachO.cpp.<br>
<br>
           // "main bin spec" (main binary specification) data payload is<br>
           // formatted:<br>
           //    uint32_t version       [currently 1]<br>
           //    uint32_t type          [0 == unspecified, 1 ==<br>
kernel, 2 == user process]<br>
           //    uint64_t address       [ UINT64_MAX if address not specified ]<br>
           //    uuid_t   uuid          [ all zero's if uuid not specified ]<br>
           //    uint32_t log2_pagesize [ process page size in log<br>
base 2, e.g. 4k pages are 12.  0 for unspecified ]<br>
<br>
<br>
So it looks like there are situations where we consider all-zero UUIDs<br>
as invalid.<br>
<br>
I guess that means we either have to keep IsValid() definition as-is,<br>
or make ObjectFileMachO check the all-zero case itself. (Some middle<br>
ground may be where we have two SetFromStringRef functions, one which<br>
treats all-zero case specially (sets m_num_uuid_bytes to 0), and one<br>
which doesn't). Then clients can pick which semantics they want.<br>
<span class=""><br>
<br>
> 1. A variable-length UUID likely incurs an extra heap allocation.<br>
</span>Not really. If you're happy with the current <=20 limit, then you can<br>
just use the existing data structure. Otherwise, you could use a<br>
SmallVector<uint8_t, 20>.<br>
<span class=""><br>
> 2. Formatting arbitrary length UUIDs as string is a bit inconvenient as you noted as well.<br>
</span>For the string representation, I would say we should just use the<br>
existing layout of dashes (after 4, 6, 8, 10 and 16 bytes) and just<br>
cut it short when we have less bytes. The implementation of that<br>
should be about a dozen lines of code.<br>
<br>
The fact that these new UUIDs would not be real UUIDs could be solved<br>
by renaming this class to something else, if anyone can think of a<br>
good name for it (I can't). Then the "real" UUIDs will be just a<br>
special case of the new object.<br>
<span class="HOEnZb"><font color="#888888"><br>
pl<br>
</font></span></blockquote></div><br></div>