How would the ObjectFile API's that take or return UUID's work in this case?

Jim


> On Nov 28, 2017, at 11:44 AM, Zachary Turner via lldb-commits 
> <lldb-commits@lists.llvm.org> wrote:
> 
> Also worth pointing out that when you write things this way, this UUID<n> 
> class can be part of a larger structure that matches the record layout of a 
> header or section in a binary file, and then you can just memcpy over the 
> class and your'e good to go.  For example you could have
> 
> ```
> struct MachOHeader {
>    ...
>    ...
>    UUID<16> uuid;
>    ...
> };
> 
> MachOHeader H;
> ::memcpy(&H, File, sizeof(H));
> File += sizeof(H);
> ```
> 
> and you could do the same for some ELF structure.
> 
> ```
> struct ElfHeader {
>    ...
>    ...
>   union {
>     UUID<20> BuildId;
>     UUID<4> DebugCrc;
>   }
>    ...
> }; 
> 
> ElfHeader H;
> ::memcpy(&H, File, sizeof(H));
> File += sizeof(H);
> ```
> 
> And everything just works.
> 
> On Tue, Nov 28, 2017 at 11:36 AM Zachary Turner <ztur...@google.com> wrote:
> Eh, that actually just makes me think the compiler *can* check it.  For 
> example, right now you can have mach-o files with 20 byte UUIDs.  But just in 
> the code, not in practice.  You could have a bug in your code that 
> accidentally wrote the wrong number of bytes from a dynamic buffer.  
> 
> You could enforce this at the compiler level by saying:
> 
> class ObjectFileMachO {
>   UUID<16> uuid;
> };
> 
> Not only is this more correct, but it is less error prone and is also nice 
> documentation to the reader who may be just learning about MachO that this 
> UUID is always 16 bytes.
> 
> For the case of ELF, it sounds like you either have a 20 byte UUID or a 4 
> byte UUID, but never both, and never any other size.  That makes me think of:
> 
> class ObjectFileELF {
>   union {
>     UUID<20> BuildId;
>     UUID<4> DebugCrc;
>   }
> };
> 
> And now the person reading this code can immediately tell that there will 
> either be one or the other, and depending on which one it is, he/she already 
> knows something about it, like how many bytes it is and what it represents.
> 
> To me this is much more clear than:
> 
> class ObjectFileELF {
>   // This might not actually be a build id, and it could be a variable size, 
> and you also have to be careful
>   // not to put some strange number of bytes in it that we don't recognize, 
> but it's up to the user
>   // to know under what circumstances it should be a certain number of bytes, 
> and you should also always
>   // be careful ensure that there's no buffer overruns since you'll be 
> working with dynamically sized buffers
>   // and the compiler can't warn you when you're doing something wrong.
>   UUID BuildIdOrDebugCrc;
> };
> 
> On Tue, Nov 28, 2017 at 11:06 AM Greg Clayton <clayb...@gmail.com> wrote:
> 
>> On Nov 28, 2017, at 10:24 AM, Zachary Turner <ztur...@google.com> wrote:
>> 
>> 
>> 
>> On Tue, Nov 28, 2017 at 10:18 AM Greg Clayton <clayb...@gmail.com> wrote:
>>> On Nov 27, 2017, at 10:11 PM, Zachary Turner <ztur...@google.com> wrote:
>>> 
>>> As an aside, I don't really like this class.  For example, You can 
>>> currently assign a UUID[16] to a UUID[20].  That doesn't make a lot of 
>>> sense to me.
>> 
>> What about an invalid UUID[0] being assigned with a valid UUID[16] or 
>> UUID[20]? Why doesn't this make sense? I don't follow.
>> 
>> Nothing is invalid, I just think it's better and expresses the intent more 
>> clearly if you can only assign between UUIDs of the same size.  For example, 
>> If the UUID class were templated on size, then there would not even be such 
>> thing as a UUID[0] or a "universally invalid UUID".  There would be an 
>> "invalid 16-byte UUID" and an "invalid 20-byte UUID", and those would be 
>> different things.
>>  
>> 
>>> 
>>> As a future cleanup, I think this class should probably be a template such 
>>> as UUID<N>, and then internally it can store a std::array<uint8_t, N>.  And 
>>> we can static_assert that N is of a known size if we desire.
>> 
>> UUID values are objects contained as members inside of other objects. They 
>> all default to start with no preconceived notion of what the UUID should be. 
>> IMHO the UUID class is just fine and needs to be able to represent any UUID, 
>> from empty uninitialized ones, and be able to be assigned and changed at 
>> will.
>> 
>> 
>> Is there ever a use case for changing the number of bytes in a UUID?  If 
>> you're working with 16-byte UUIDs, does it ever actually happen that now you 
>> have a 20-byte UUID?  Can you imagine a use case currently where an N-byte 
>> UUID is being compared against an M-byte UUID in a real-world scenario?  If 
>> the answer is no, then it may as well be enforced by the compiler. 
> 
> The ObjectFile class has a "UUID m_uuid;" member that any object file can 
> fill in. Right now mach-o files have 16 byte UUIDs. ELF files can have 20 
> bytes UUIDs (build ID) or 4 byte UUIDs (debug info CRC if no build ID is 
> around, and these are current represented as 20 byte UUIDs with just the 
> first 4 bytes filled in. So no, we can't enforce this using the compiler. I 
> don't see a need to change way from a byte buffer that has the max number of 
> bytes needed for any currently supported UUID (20 right now). 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to