Re: [Boost-users] [review][fixed_string] FixedString review starts today

From: Peter Dimov (pdimov_at_[hidden])
Date: 2019-11-29 07:14:08

Zach Laine wrote:

> Ok, I understand your point a bit better now I think.  Is it the 
> unboundedly-large nature of a NTBS that has you concerned?  That is, do 
> you think that op+=(char) should assert and op+=(char const *) should 
> throw?

No, I prefer all of them consistently throwing, even when the check is O(1). 
My primary concern is not the loss of efficiency due to double checking, 
it's to avoid buffer overflow exploits.

Yes, this will be less efficient if you += characters (but not by so much as 
one might think), and yes, you can write code that will be correct if op+= 
doesn't check. (Since operating on chars destroys optimizations due to 
aliasing, the inner loop improves from

.L5:
        add     rax, 1
        movzx   ecx, BYTE PTR [rdx]
        cmp     rax, 511
        ja      .L11
        add     rdx, 1
        mov     QWORD PTR [rdi], rax
        mov     BYTE PTR [rdi+7+rax], cl
        cmp     rsi, rdx
        jne     .L5

https://godbolt.org/z/Z37Pmx

to

.L3:
        movzx   ecx, BYTE PTR [rdx+rax]
        add     rax, 1
        mov     QWORD PTR [rdi], rax
        mov     BYTE PTR [rdi+7+rax], cl
        cmp     r8, rax
        jne     .L3

https://godbolt.org/z/CXHRKQ

which is better, but far from optimal.)

I were implementing this class, I would always check in op+= (and append 
etc) even if the specification says "Expects" instead of "Throws", I'd just 
__builtin_trap instead of throwing. Otherwise, this is strcat all over 
again, and there's a reason there was a big concerted push against this 
style in/by Microsoft and elsewhere.