Hi Benjamin, well, it might not crash actually, that's the issue with undefined behavior, so I'll just add the assert.
Thanks for the link :) Matthieu. 2011/4/30 Benjamin Kramer <[email protected]> > > On 30.04.2011, at 12:18, Matthieu Monrocq wrote: > > > The llvm::StringRef class has a constructor taking a const char* > parameter. > > > > 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. > > > > 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. > > > > As far as I see it, there are two ways to deal with this: > > - 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 > > - 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. > > > > I've used the second approach in the patch enclosed (which gmail > thoroughly refused to. I have not measured the performance impact though. > > Hi Matthieu, > > We had this discussion before < > http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20090928/088120.html > >. > > 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. > > An assert would be ok, but I don't think it's needed because strlen(NULL) > is going to crash anyway. > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
