Following Benjamin's remark, here is a 3rd iteration of the patch:<br><br>- allow  Data  to be null as it is the current behavior and unknown amount of code relies on this property meaning that the StringRef is "void", but assert that the pointer provided to  strlen  is non-null to avoid undefined behavior and provide a clear diagnosis in case this condition is violated<br>
- since  Data  can be null, provide a specialized version of  memcmp  (like it was done for  min  and  max) that allows either pointer to be null whenever the number of bytes to compare is zero<br><br>Arguably, the static function could be named otherwise (rawcompare ?) and the calls to memcmp replaced.<br>
<br>This allows a straightforward use of the compare methods that does not require the caller to check that either is null beforehand (which is quite delicate in a call to  sort  or similar algorithm).<br><br>It should be transparent for those using the StringRef correctly already.<br>
<br><br>Please review.<br>Matthieu.<br><br><br><div class="gmail_quote">2011/4/30 Matthieu Monrocq <span dir="ltr"><<a href="mailto:matthieu.monrocq@gmail.com">matthieu.monrocq@gmail.com</a>></span><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
Hi Benjamin,<br><br>well, it might not crash actually, that's the issue with undefined behavior, so I'll just add the assert.<br><br>Thanks for the link :)<br><font color="#888888">Matthieu.</font><div><div></div>
<div class="h5"><br><br><div class="gmail_quote">2011/4/30 Benjamin Kramer <span dir="ltr"><<a href="mailto:benny.kra@googlemail.com" target="_blank">benny.kra@googlemail.com</a>></span><br>
<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div><div></div><div><br>
On 30.04.2011, at 12:18, Matthieu Monrocq wrote:<br>
<br>
> The  llvm::StringRef  class has a constructor taking a  const char*  parameter.<br>
><br>
> This constructor is extremely simple, and I am afraid too simple. It directly invokes  ::strlen  on the parameter, without checking whether or not the pointer is null or not.<br>
><br>
> Unfortunately as many C functions, strlen is not required by the standard to check its input, and indeed popular implementations assume that the input is not null, which results in undefined behavior.<br>
><br>
> As far as I see it, there are two ways to deal with this:<br>
> - using an assert, to check that the input is non-null. It does not slow-down the program built with asserts disabled, but does not allow us to invoke StringRef on null pointers<br>
> - using a simple inlined test (ternary operator ?:) to either invoke strlen or set the length to 0. Makes migrating from  const char*  to  llvm::StringRef  easier.<br>
><br>
> I've used the second approach in the patch enclosed (which gmail thoroughly refused to. I have not measured the performance impact though.<br>
<br>
</div></div>Hi Matthieu,<br>
<br>
We had this discussion before <<a href="http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20090928/088120.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20090928/088120.html</a>>.<br>


<br>
The result was that we don't want to have a NULL check in StringRef's ctor because it would slow down many users of StringRef and instead code that passes NULL to StringRef should be fixed.<br>
<br>
An assert would be ok, but I don't think it's needed because strlen(NULL) is going to crash anyway.<br>
<br>
</blockquote></div><br>
</div></div></blockquote></div><br>