[review][fixed_string] Andrzej's feedback

classic Classic list List threaded Threaded
28 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Re: [review][fixed_string] Andrzej's feedback

Boost - Dev mailing list
Folks, we are way overthinking the behavior of resize() over
capacity(). It throws for a very simple reason. The most obvious and
common optimization for highly concurrent network programs is simply
to avoid the global allocator. And the obvious and common mitigation
for resource exhaustion attacks is to limit the amount of data that an
I/O handler is allowed to process.

Using a fixed string as a temporary buffer for an algorithm satisfies
both of these goals, and it does so without the caller having to write
a bunch of code to check sizes in advance.

Thanks

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][fixed_string] Andrzej's feedback

Boost - Dev mailing list
czw., 5 gru 2019 o 14:39 Vinnie Falco via Boost <[hidden email]>
napisał(a):

> Folks, we are way overthinking the behavior of resize() over
> capacity(). It throws for a very simple reason. The most obvious and
> common optimization for highly concurrent network programs is simply
> to avoid the global allocator. And the obvious and common mitigation
> for resource exhaustion attacks is to limit the amount of data that an
> I/O handler is allowed to process.
>
> Using a fixed string as a temporary buffer for an algorithm satisfies
> both of these goals, and it does so without the caller having to write
> a bunch of code to check sizes in advance.
>

Thanks you. This is the kind of information that I was after.

I would like to know more. Given that fixed_string does not have operator+,
as well as the description above, I am inclined to think that it is not
treated as a string, but as a buffer of characters. Next question is, why
is a vector with fixed capacity not enough for this purpose? Do you ever
have a need to call `.find_first_not_of()` in your use cases?

Regards,
&rzej;

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][fixed_string] Andrzej's feedback

Boost - Dev mailing list
On Thu, Dec 5, 2019 at 6:07 AM Andrzej Krzemienski <[hidden email]> wrote:
> I would like to know more. Given that fixed_string does not have operator+,
> as well as the description above, I am inclined to think that it is not treated
> as a string, but as a buffer of characters. Next question is, why is a vector
> with fixed capacity not enough for this purpose? Do you ever have a need
> to call `.find_first_not_of()` in your use cases?

When dealing primarily with text based protocols: HTTP, URL, JSON,
etc... you usually want to perform string operations. I agree that
"drop-in replacement for std::string" is not a precise description for
the purpose of the library, but it is partially correct. If you have
code that currently uses std::string to perform calculations, and you
want to impose a limit on the amount of data it can process while
simultaneously avoiding memory allocations, a fixed_string is going to
be easier to integrate than vector<char>.

Thanks

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][fixed_string] Andrzej's feedback

Boost - Dev mailing list
czw., 5 gru 2019 o 15:11 Vinnie Falco <[hidden email]> napisał(a):

> On Thu, Dec 5, 2019 at 6:07 AM Andrzej Krzemienski <[hidden email]>
> wrote:
> > I would like to know more. Given that fixed_string does not have
> operator+,
> > as well as the description above, I am inclined to think that it is not
> treated
> > as a string, but as a buffer of characters. Next question is, why is a
> vector
> > with fixed capacity not enough for this purpose? Do you ever have a need
> > to call `.find_first_not_of()` in your use cases?
>
> When dealing primarily with text based protocols: HTTP, URL, JSON,
> etc... you usually want to perform string operations. I agree that
> "drop-in replacement for std::string" is not a precise description for
> the purpose of the library, but it is partially correct. If you have
> code that currently uses std::string to perform calculations, and you
> want to impose a limit on the amount of data it can process while
> simultaneously avoiding memory allocations, a fixed_string is going to
> be easier to integrate than vector<char>.
>

Maybe we need a concept that would help draw the line clearly?

Regards,
&rzej;

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][fixed_string] Andrzej's feedback

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list

> Such method is called operator[]. I understand you want that name for
> the checked version, but that is not the case for historical reasons
> (for which I'm glad).
>
> And again, where check is needed, I will have it written myself and
> the way I want it (which may not involve throwing an exception). Where
> I don't have it written, I don't want it, at all, and I'm responsible
> for the consequences. I would prefer if such use pattern didn't
> require me jumping through hoops.
Well in a better world you'd use .at(i) instead of [i] because YOU know
what you are doing. Beginners don't. Advanced people neither, as can be
seen by CVEs.
> It is not. It doesn't make your code correct, it doesn't make it work,
> it doesn't help you fixing the issue. It only prevents your code from
> crashing, which is a bad thing (because a crash - with a backtrace -
> is better).
You keep assuming that an OOB access leads to a crash. This is NOT the
case. An OOB access is UB and a security risk.
> Thing is, you can't prepare a meaningful error in main(). You don't
> have a backtrace at that point and you don't know which of the
> thousands of at() calls threw. Sure, your application won't crash via
> signal, but, as I said, that is a bad thing.
I thought at least on Windows you got unhandled_exception_filter or so.
Well then don't use an exception but a signal or std::abort. Or a
stack-trace enhanced exception (e.g.
https://sourceforge.net/p/stacktrace), still not perfect of course.
Anyway at least debuggers can break on exceptions, so you got that.
> I don't want that marginal performance drop, especially given that I
> get nothing in return.
Again: You get security. At least std::abort, but never continue!
> You could terminate the connection and things like that, but
> ultimately that exception doesn't help you fixing the problem. When
> connections are dropping like flies on your production server and you
> have no idea where to fix the problem, I'd prefer to get just one
> backtrace of a crash instead and then fix the bug in a matter of minutes.
Attach a debugger, break on throw.
Alternative: Have users craft packets to force a buffer-overflow and
read your secrets or take over your machine. Great!
> With asserts enabled you'll get a crash at the point of error.
> Hopefully, you'll discover the bug soon enough during testing. But
> even in release builds, when asserts are disabled, chances are high
> the crash will still point you to the problematic code. Of course,
> memory access bugs are nasty and painful to deal with, but I honestly
> didn't have to deal with one for quite a long time now, even though
> I've never used at(). You're more likely to mess up memory when
> working with raw pointers.
Sure, then have asserts in release mode. Optimized away in 90% of the
cases but rest assured that no-one takes over your machine. I don't care
if exceptions are used, anything preventing execution of the next
instruction is fine.



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

smime.p7s (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [review][fixed_string] Andrzej's feedback

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 6/12/2019 03:11, Vinnie Falco wrote:
> When dealing primarily with text based protocols: HTTP, URL, JSON,
> etc... you usually want to perform string operations. I agree that
> "drop-in replacement for std::string" is not a precise description for
> the purpose of the library, but it is partially correct. If you have
> code that currently uses std::string to perform calculations, and you
> want to impose a limit on the amount of data it can process while
> simultaneously avoiding memory allocations, a fixed_string is going to
> be easier to integrate than vector<char>.

vector and/or static_vector combined with Boost.StringAlgorithm?

Maybe we should just be making a more modern static_vector rather than a
fixed_string.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][fixed_string] Andrzej's feedback

Boost - Dev mailing list
On Thu, Dec 5, 2019 at 1:53 PM Gavin Lambert via Boost
<[hidden email]> wrote:
> Maybe we should just be making a more modern static_vector rather than a
> fixed_string.

Add CharTraits to boost::static_vector? Sure! I'll let you write that
up and submit it as a pull request...

Regards

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][fixed_string] Andrzej's feedback

Boost - Dev mailing list
On 6/12/2019 11:16, Vinnie Falco wrote:
> On Thu, Dec 5, 2019 at 1:53 PM Gavin Lambert wrote:
>> Maybe we should just be making a more modern static_vector rather than a
>> fixed_string.
>
> Add CharTraits to boost::static_vector? Sure! I'll let you write that
> up and submit it as a pull request...

It doesn't need them.  Boost.StringAlgorithm is the part that deals with
char traits.

And it already supports duck-typed containers.  The only thing possibly
missing is constexpr support.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
12