[BUG] - Several potential bugs found using an automated code analysis tool

[zmandel]

Internal/External
External

Area
Other

Summary
I ran a novel AI-based code-analysis tool on the entire cardano node directory (I'm the tool maker, its not released yet) and found 14 potential issues.

Steps to reproduce
I didn't want to create an issue for each. The issue list is in this Google Sheet.
I also uploaded it as issues cardano-node.csv but the CSV could be outdated as the Google Sheet is editable.

Scan Date: 7/27/2024
Scanned code: https://github.com/IntersectMBO/cardano-node/tree/1e6f75101b5a637169efd8cb29026ad23ba4a427/cardano-node/src/Cardano/Node

If this feedback is useful, I can gladly run the tool on other directories, even on the entire repository.

[kevinhammond]

Thank you!

[erikd]

One comment from our internal Slack channel that may be of interest to you:

I can’t comment on other items, as I have not looked at them, but item 4 seems bogus, unfortunately: The AI didn’t recognize bracket.

[zmandel]

indeed, the AI initially gave me 30 potential issues and I manually removed the bogus ones, I didn't notice that one (im new to haskell)

…

On Wed, Jul 31, 2024, 10:36 PM Erik de Castro Lopo ***@***.***> wrote: One comment from our internal Slack channel that may be of interest to you: I can’t comment on other items, as I have not looked at them, but item 4 seems bogus, unfortunately: The AI didn’t recognize bracket. — Reply to this email directly, view it on GitHub <#5926 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABBJ2JZ2H5DCCSSN74MHMLLZPGUKTAVCNFSM6AAAAABLSFFSY6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENRRHA4TMOJTGM> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[erikd]

The bracket family of functions in Haskell are of the form:

bracket setup shutdown action

Where the bracket function runs setup , then action and then shutdown (regardless of whether action threw an exception). Ie, shutdown is always run.

[zmandel]

@erikd @kevinhammond ahh, It seems you have confused the meaning of the last column "More Details" on that issue in line number 4. That code that uses "bracket" is actually the suggestion that my AI came up with. The actual Cardano code does not use a bracket and leaks the handle. I also updated the file and line number where it occurs (it was on shutdown.hs)

[coot]

For the network related things:

• Ad 2: setBlockForging :: [BlockForging m blk] -> m () - it doesn't return any meaningful result. The function does not raises any meaningful exception either, see - it could raise an IOError (e.g. out of memory, etc), but such exception would also be thrown to other threads.

• Ad 5: This is correct: if the node cannot get information about IPv{4,6} addresses it should fail, hence no special error handling is required.

• Ad 10: this is about this code snippet. The code is correct, the function returns error if both configurations couldn't be parsed - although it only reports the new formatting error.

• Ad 14: the AI is right that this might lead to different behaviour if the exception is modified or hierarchy is changed. However this is in NonP2P code path, which will be retired. Only for that reason I'd mark it as no-op.

• Ad 15: I don't see any type casting in the Cardano.Node.Tracing.Tracers.P2P module.

[github-actions]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 120 days.