On Fri, Feb 16, 2018 at 11:19 AM, Martin Sebor <mse...@gmail.com> wrote:
> On 02/16/2018 05:43 AM, Nick Clifton wrote:
>>
>> Hi David,
>>
>> Attached is a revised version of the patch which I hope addresses all
>> of your (very helpful) comments on the v3 patch.
>>
>> OK to apply once the sources are back on stage 1 ?
>
>
> Thanks (also) for adding the documentation!
>
> I have just one minor observation/suggestion for the escaped_string
> class. Feel free not to change anything, it's just a possible gotcha
> if the class were to become more widely used than it is now.
>
> Since the class manages a resource it should ideally make sure it
> doesn't try to release the same resource multiple times. I.e., its
> copy constructor and assignment operators should either "do the right
> thing" (whatever you think that is) or be made inaccessible (or declared
> deleted in C++ 11).
>
> For example:
>
> {
> escaped_string a;
> a.escape ("foo\nbar");
>
> escaped_string b (a);
> // b destroys its m_str
> // double free in a's destructor here
> }
>
> (Some day GCC will have a warning to help detect this pitfall.)
-Wdeprecated-copy, which I recently added to -Wall, will warn if such
a class is actually copied. Though I'm not sure it will stay in
-Wall, or keep that name.
Jason
