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: RPT_ALINKS / RPT_LINKS + associated l->links evalutation #502

Open
mkmer opened this issue Feb 20, 2025 · 4 comments
Open

app_rpt: RPT_ALINKS / RPT_LINKS + associated l->links evalutation #502

mkmer opened this issue Feb 20, 2025 · 4 comments

Comments

@mkmer
Copy link
Collaborator

mkmer commented Feb 20, 2025

While we have addressed the string size for RPT_ALINKS and RPT_LINKS, l->links are still limited to [5120]-1 characters.

In a large network (somewhere around 510 nodes depending on node name sizes) a network node could receive a link list larger than 5120 which will be truncated.

While not critical, evaluation of how the strings are stored and alternative methods should be reviewed at some point.
Reference PR #474 and PR #470

@InterLinked1
Copy link
Member

Personally, I think this is a great candidate for using an ast_str. A fixed size buffer for something that can vary with essentially no bounds doesn't seem appropriate here.

@mkmer
Copy link
Collaborator Author

mkmer commented Feb 23, 2025

Also, while evaluating the l->linklist sizes, many intermediate local variables sizes (eg: cmd, src, tmp) should also be reviewed. There appears to be inconsistent sizes (eg: 512 vs 5120, 30 vs 300) between functions the process the "same" messages.

@Allan-N
Copy link
Collaborator

Allan-N commented Feb 23, 2025

Mike and I were both looking at the array sizes and they are all over the map. In many places you will find an array sized to MAXLINKLIST (5120). Why this size? Was it supposed to be 10 blocks of 512 bytes? What was worse was that this size was used both for a character (string) array and for an array of pointers.

I agree that we need to move the code base away from using fixed sized arrays in places that are dependent on the number of connections or linked nodes and ast_str looks like it would be a good fit (even better if we can figure out a nice "initial" size ... and maybe even adjust that size based on usage).

Is there an ast_ptr equivalent for managing arrays of pointers (e.g. exploding a string into argv[] arguments, exploding a string of adjacent nodes into list of nodes)?

@InterLinked1
Copy link
Member

Mike and I were both looking at the array sizes and they are all over the map. In many places you will find an array sized to MAXLINKLIST (5120). Why this size? Was it supposed to be 10 blocks of 512 bytes? What was worse was that this size was used both for a character (string) array and for an array of pointers.

That's kind of reflective of what I've observed in app_rpt over the years. There is often no rhyme or reason to the way buffer sizes are the way they are.

Any time a buffer size is a nice roundish number, like 100, it's a good chance that's a completely arbitrary size somebody pulled out of a thin air.

I agree that we need to move the code base away from using fixed sized arrays in places that are dependent on the number of connections or linked nodes and ast_str looks like it would be a good fit (even better if we can figure out a nice "initial" size ... and maybe even adjust that size based on usage).

Is there an ast_ptr equivalent for managing arrays of pointers (e.g. exploding a string into argv[] arguments, exploding a string of adjacent nodes into list of nodes)?

Arrays by definition are fixed size, so that case might be a bit different. Is there an example that comes to mind?

For a list of nodes, a linked list is probably the closest thing I can think of. They're already used in some places, so in the places we're still trying to use arrays, maybe we should move those to linked lists as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants