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>Matthieu.<br><br><div class="gmail_quote">2011/4/30 Benjamin Kramer <span dir="ltr"><<a href="mailto:benny.kra@googlemail.com">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 class="h5"><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>