Modernize Humminbird internals

[robertlipe]

• Modernize Humminbirds internals

[GPSBabelDeveloper]

I gave up on this last night. It was driving me crazy as you can tell from my hit-and-miss attempts to debug it in CI.

I first missed from char* interfaces, but later moved to those. strnlen might be the ticket. My goal was to get rid of xstrndup as this is about the only caller left. My final version last night went back to using xstrndup and I was still having luck.

I need to go back and be sure that I'm using the struct sizes I should be using and such. I probaby got carried away with a yank and put and have they wrong field size or something.

I'll pick it back up tonight after dr. appts. Of course, if that function goes back to being a one-liner (and I'd forgotten about QByteArray having "better" ctors here) then this function goes away. I don't know why I swapped the arguments. Maybe I shouldn't have been programming at all last night. I was able to get some failures with the debug malloc on MacOS, but it consistently worked on MacOS and not any other system that we care about, so it became quite the frustrating hunt.

And, yes, while it's my nature to count clock cycles from so many years of doing systems software, I'll readily conced that humminbird's speed is veery low on my list of reasons to not be sleeping.

I'll pick it back up. Thanx for the hint and the pep-talk that this shouldn't be NEARLY as time-consuming as it was.

On Wed, Aug 14, 2024 at 8:27 AM tsteven4 @.***> wrote:

@.**** commented on this pull request.

In humminbird.cc https://github.com/GPSBabel/gpsbabel/pull/1322#discussion_r1716927252:

@@ -247,24 +284,20 @@ HumminbirdBase::humminbird_read_wpt(gbfile* fin)

auto* wpt = new Waypoint;

• // Could probably find a way to eliminate the alloc/copy.
• char* s = xstrndup(w.name, sizeof(w.name));
• wpt->shortname = s;
• xfree(s);
• wpt->shortname = QStringFromRaw(sizeof(w.name), w.name);

Isn't this all we need here? If so can we eliminate QStringFromRaw or replace its contents along these lines?

wpt->shortname = QByteArray(w.name, qstrnlen(w.name, sizeof(w.name)));

You could use QByteArray::fromRawData instead but I suggest saving that copy is noise and prone to misuse (source must continue to exist, result not '\0' terminated).

— Reply to this email directly, view it on GitHub https://github.com/GPSBabel/gpsbabel/pull/1322#pullrequestreview-2238167818, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC3VAD2B56ETS5UIB4RFDJLZRNLLTAVCNFSM6AAAAABMPL5BH6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEMZYGE3DOOBRHA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[codacy-production[bot]]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
:white_check_mark: -0.03%	:white_check_mark: 76.92%

Coverage variation details

| | Coverable lines | Covered lines | Coverage | | ------------- | ------------- | ------------- | ------------- | | Common ancestor commit (4e017862e159f75e43a0d254564b5e9c2c6c0b37) | 27552 | 17233 | 62.55% | | | Head commit (6c6c562ddaeafa5e6f03086119d3ecd1c23a79e7) | 55109 (+27557) | 34455 (+17222) | 62.52% (**-0.03%**) | **Coverage variation** is the difference between the coverage for the head and common ancestor commits of the pull request branch: ` - `

Diff coverage details

| | Coverable lines | Covered lines | Diff coverage | | ------------- | ------------- | ------------- | ------------- | | Pull request (#1322) | 26 | 20 | **76.92%** | **Diff coverage** is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: `/ * 100%`

See your quality gate settings    Change summary preferences

_{Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more}