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

[EdB]

Le 2014-08-20 16:03, Francisco Jerez a écrit :
> 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.

Sure, but I don't think people will reserve this extra capacity.
especially if they use the char* and the std::sting constructor.
I thinks c_str will (should) mostly be use for debug purpose,
they won't change capacity before displaying the string.

> 
>> 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
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev