Refactored prival from string for performance

[Griezn]

This PR addresses issue #428 by significantly reducing allocations and execution time, as demonstrated by the run-performance-test-prival results.

Updated get_x_from_buffer functions to correctly utilize the length parameter and ensure function test correctness.

The check I removed failed. I don't know exactly why it the check didn't exist in the corresponding test of facility so I removed it.

[codecov]

Codecov Report

Attention: Patch coverage is 77.77778% with 4 lines in your changes missing coverage. Please review.

Project coverage is 90.48%. Comparing base (fb0d2e0) to head (f894f7f).
Report is 1 commits behind head on latest.

Files with missing lines	Patch %	Lines
src/strhelper.c	66.66%	1 Missing and 2 partials ⚠️
src/severity.c	75.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           latest     #437      +/-   ##
==========================================
- Coverage   90.86%   90.48%   -0.38%     
==========================================
  Files          47       47              
  Lines        4389     4373      -16     
  Branches      586      585       -1     
==========================================
- Hits         3988     3957      -31     
- Misses        269      281      +12     
- Partials      132      135       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[goatshriek]

Thanks very much for putting this change together! My apologies for the delay in getting it reviewed.

There is just one change that needs to be made in order to properly preserve the cause of an error, and once that is done this should be ready to go!

[goatshriek]

Just one more header fixup and I believe this will be ready.

further changes needed

[goatshriek]

My apologies for changing this review response, but in reviewing the coverage report and seeing the error check as only partially covered, I believe the logic needs to be adjusted slightly as mentioned in the comment. This should both correct the coverage and also make the error return path more clear and intentional.

[Griezn]

So what I did now is, I solved issue #438 also with this pr. With this fix there are no allocations performed by the function so from my perspective those tests become irrelevant. This also allowed me to remove the check I added to see if there already was an error.

But I think the issue of errors being overwritten can still occur in other places so maybe this is a new issue.

[goatshriek]

I think this is a great approach! There are a few other tests that are failing now as they were expecting memory allocation failures - you can either remove these completely or simply change the expected results to be no error.

[goatshriek]

The Windows builds are failing because strncasecmp isn't completely portable, it seems. I can think of two ways forward right now. The first is a bit simpler: to add a custom implementation of strncasecmp to the strhelper files (probably with a different function name) and use this. The second is more involved but will be the end state of the project: add a configuration step to detect if the build environment has a library function that can be used (strncasecmp, _strnicmp, etc) and uses this if it is available, and otherwise falls back to an embedded default.

Since the second option will require some cmake work and the addition of several more source files and config_ functions (see the portability docs for more info), I would just stick with the first option in this change. I can create a second issue to track the second part of this, since using the system library would reduce library size and potentially improve performance when available.

[goatshriek]

Thanks once again for putting this change together, and sticking with it through all of the requested changes!