Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

app_rpt.c: Refactor handle_link_data() and handle_remote_data() #474

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

mkmer
Copy link
Collaborator

@mkmer mkmer commented Jan 29, 2025

This is a follow on to PR #470 which should not be delayed. Intent is to clean up the stuff I notice while hunting down the truncated message.

Eliminate copy of malloc char* to fixed length char*
update sprintf() to snprintf()
rpt_link.c: Calculate buffer size and malloc buffers. add function __get_buffer_size() to calculate buffer size required. add node count to __mklinklist()
add macro for OBUFSIZE and BUFSIZE calculation
eliminate finddelim() call as __mklinklist() now counts links.

apps/app_rpt/rpt_link.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_link.c Show resolved Hide resolved
apps/app_rpt/rpt_link.c Show resolved Hide resolved
apps/app_rpt/rpt_link.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_link.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_link.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_link.c Outdated Show resolved Hide resolved
if (l->mode > 1)
continue; /* dont report local modes */
if (l->linklist[0]) {
buffer_size += (strlen(l->linklist) + strlen(l->name) + 3); /*+3: 2 for the commas, 1 for mode, 1 extra as the first pass has no comma*/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused about needing "3". You need 1 for the mode and 1 more for either a comma (if more nodes follow) or the end-of-line (if this was the last node).

Note: if looks like RPT_ALINKS includes an extra character so we do need 3 (mode, keyed/unkeyed, and comma/EOL)

Copy link
Collaborator Author

@mkmer mkmer Jan 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a comma appended to the end of the full list except for the first pass:
T123, "stuff from the linklist" , T345, "stuff from the next link list" -> 2 commas if more than 1 link (between T123 and T345) other wise only 1 comma.
Maybe my comment about 1 extra comma for the first pass is not clear enough, we will be 1 char larger than we need.

apps/app_rpt/rpt_link.c Outdated Show resolved Hide resolved
update sprintf to snprintf
rpt_link.c: Calculate buffer size and malloc buffers.
add function __get_buffer_size() to calculate buffer size required.
add node count to __mklinklist()
add macro for OBUFSIZE and BUFSIZE calculation
elminate finddelim() call as __mklinklist now counts links
@mkmer mkmer force-pushed the Refactor-link-message branch from ff1ee3c to 9f1050a Compare January 29, 2025 21:08
@mkmer
Copy link
Collaborator Author

mkmer commented Jan 29, 2025

I will squash into a single commit once we are done reviewing it.

apps/app_rpt/rpt_link.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_link.c Show resolved Hide resolved
apps/app_rpt/rpt_link.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_link.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_link.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_telemetry.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_link.h Outdated Show resolved Hide resolved
apps/app_rpt.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_cli.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_cli.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_cli.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_link.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_telemetry.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_telemetry.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_telemetry.c Outdated Show resolved Hide resolved
@mkmer
Copy link
Collaborator Author

mkmer commented Jan 30, 2025

Fixing up finddelim may not be possible. I think some of the instances are searching through non null terminated values (but I can't be sure right now). With numbers like 3 in the limit while strs[100].
The finddelim is all in one commit we can easily back out if we think it should hold off for another PR.

@Allan-N
Copy link
Collaborator

Allan-N commented Jan 30, 2025

I'll take a look at the finddelim call sites tomorrow.

@Allan-N
Copy link
Collaborator

Allan-N commented Jan 30, 2025

FYI : in .../asterisk/include/asterisk/utils.h I just found :

#define ARRAY_LEN(a) (size_t) (sizeof(a) / sizeof(0[a]))

So, when needed, we can use ARRAY_LEN(strs)

apps/app_rpt/rpt_cli.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_cli.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_config.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_config.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_config.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_telemetry.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_telemetry.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_telemetry.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_telemetry.c Outdated Show resolved Hide resolved
apps/app_rpt/rpt_utils.c Outdated Show resolved Hide resolved
@Allan-N
Copy link
Collaborator

Allan-N commented Jan 30, 2025

Looks like chan_echolink.c, chan_tlb.c, and chan_voter have their own copy/version of findelim() that all need the same help.

@mkmer
Copy link
Collaborator Author

mkmer commented Jan 30, 2025

Looks like chan_echolink.c, chan_tlb.c, and chan_voter have their own copy/version of findelim() that all need the same help.

I saw that when I was making the move.
I think (and will) do the channels in separate PR. 1 for each channel.
This PR is getting to be one of those threads you don't want to pull :) Just keeps going and going.

@mkmer mkmer added the code quality Improvments around code quality without functional changes label Jan 30, 2025
@mkmer mkmer force-pushed the Refactor-link-message branch from 0d63fd5 to 76c4416 Compare January 30, 2025 17:39
apps/app_rpt.c Outdated Show resolved Hide resolved
apps/app_rpt.c Show resolved Hide resolved
apps/app_rpt/rpt_link.c Show resolved Hide resolved
apps/app_rpt/rpt_link.c Show resolved Hide resolved
apps/app_rpt/rpt_telemetry.c Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Improvments around code quality without functional changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants