[Mesa-dev] [PATCH 3/3] clover: unsure compat::string is \0 terminated

[Francisco Jerez]

EdB <edb at sigluy.net> writes:

> Each time you call c_str() it will grow up, may be you could check if 
> the string is already \0 terminated before adding it.

Nope, that's not how it works.  Every time c_str() is called the size of
the underlying array is forced to at least size-of-the-actual-string +
1, so nothing will happen if the array is already big enough.

> The way we do it, we use twice the memory every time a vector capacity 
> increase (before freeing the old vec) as we don't use a realloc.
> I understand c_str() should be use for debug only purpose, but may be it 
> could be a problem while debugging huge strings.
>
> Or we can keep compat::string the same and remove c_str(). If someone 
> needed it, he could use std::string operator and c_str() on it.
> At the end, the memory used is the same.
>
>
> Le 2014-08-18 14:35, Francisco Jerez a écrit :
>> EdB <edb+mesa at sigluy.net> writes:
>> 
>>> otherwise c_str() is not safe
>>> ---
>>>  src/gallium/state_trackers/clover/util/compat.hpp | 54 
>>> ++++++++++++++++++++---
>>>  1 file changed, 48 insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/src/gallium/state_trackers/clover/util/compat.hpp 
>>> b/src/gallium/state_trackers/clover/util/compat.hpp
>>> index 6f0f7cc..7ca1f85 100644
>>> --- a/src/gallium/state_trackers/clover/util/compat.hpp
>>> +++ b/src/gallium/state_trackers/clover/util/compat.hpp
>>> @@ -197,7 +197,7 @@ namespace clover {
>>>              return _p[i];
>>>           }
>>> 
>>> -      private:
>>> +      protected:
>>>           iterator _p;  //memory array
>>>           size_type _s; //size
>>>           size_type _c; //capacity
>>> @@ -306,18 +306,56 @@ namespace clover {
>>> 
>>>        class string : public vector<char> {
>>>        public:
>>> -         string() : vector() {
>>> +         string() : vector(0, 1) {
>>> +            _p[_s - 1] = '\0';
>>>           }
>>> 
>>> -         string(const char *p) : vector(p, std::strlen(p)) {
>>> +         string(const char *p) : vector(p, std::strlen(p) + 1) {
>>> +            _p[_s - 1] = '\0';
>>>           }
>>> 
>>>           template<typename C>
>>> -         string(const C &v) : vector(v) {
>>> +         string(const C &v) : vector(&*v.begin(), v.size() + 1) {
>>> +            _p[_s - 1] = '\0';
>>>           }
>>> 
>>> -         operator std::string() const {
>>> -            return std::string(begin(), end());
>>> +         void
>>> +         reserve(size_type m) {
>>> +            vector::reserve(m + 1);
>>> +         }
>>> +
>>> +         void
>>> +         resize(size_type m, char x = '\0') {
>>> +            vector::resize(m + 1, x);
>>> +            _p[_s - 1] = '\0';
>>> +         }
>>> +
>>> +         void
>>> +         push_back(char &x) {
>>> +            reserve(_s + 1);
>>> +            _p[_s - 1] = x;
>>> +            _p[_s] = '\0';
>>> +            ++_s;
>>> +         }
>>> +
>>> +         size_type
>>> +         size() const {
>>> +            return _s - 1;
>>> +         }
>>> +
>>> +         size_type
>>> +         capacity() const {
>>> +            return _c - 1;
>>> +         }
>>> +
>>> +         iterator
>>> +         end() {
>>> +            return _p + size();
>>> +         }
>>> +
>>> +         const_iterator
>>> +         end() const {
>>> +            return _p + size();
>>>           }
>>> 
>> 
>> At this point where all methods from the base class need to be 
>> redefined
>> it probably stops making sense to use inheritance instead of
>> aggregation.  Once we've done that fixing c_str() gets a lot easier 
>> (two
>> lines of code) because we can just declare the container as mutable and
>> fix up the NULL terminator when c_str() is called.  Both changes
>> attached.
>> 
>>>           const char *
>>> @@ -325,6 +363,10 @@ namespace clover {
>>>              return begin();
>>>           }
>>> 
>>> +         operator std::string() const {
>>> +            return std::string(begin(), end());
>>> +         }
>>> +
>>>           const char *
>>>           find(const string &s) const {
>>>              for (size_t i = 0; i + s.size() < size(); ++i) {
>>> --
>>> 2.0.4
>>> 
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140820/63a952ab/attachment.sig>