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

Fix "cli_strdup(): s == NULL" error with clamonacc --wait #984

Merged
merged 1 commit into from
Aug 3, 2023
Merged

Fix "cli_strdup(): s == NULL" error with clamonacc --wait #984

merged 1 commit into from
Aug 3, 2023

Conversation

rzvncj
Copy link
Contributor

@rzvncj rzvncj commented Jul 29, 2023

Issue occured when --ping # option was missing.

Fixes #443.

@rzvncj
Copy link
Contributor Author

rzvncj commented Jul 29, 2023

Looking at the code, I've seen this sort of thing happening a bit:

if (attempt_str) {
    free(attempt_str);
}

But free(NULL) should be perfectly safe (according to 7.20.3.2 of ISO-IEC 9899), so it seems like we could simplify those. Do we need to work with non-compliant compilers?

Issue occured when --ping # option was missing.

Fixes #443.
@rzvncj
Copy link
Contributor Author

rzvncj commented Jul 29, 2023

Is there anything I can do to fix the Windows PR check? I remember it didn't work on my previous (now merged) PR either.

@micahsnyder
Copy link
Contributor

Is there anything I can do to fix the Windows PR check? I remember it didn't work on my previous (now merged) PR either.

Nothing you can easily do. I would not be surprised if we need to upgrade to a newer version of the lukka/run-cmake action to get it working again.

But the latest version has a different interface and also requires using a CMakePresets.json file.
Since we support all sorts of builds on Linux, Mac, Windows, and FreeBSD, setting up such a thing seems daunting. I need to do some reading and testing to try to solve that.

Don't worry about it for now. We do additional testing on Windows 10, 7, 32bit, and 64bit on our internal Jenkins and I will verify that all is well there for each PR. And also since this is only touching Clamonacc which doesn't exist for Windows -- even less need to worry. :)

@micahsnyder micahsnyder merged commit 7194388 into Cisco-Talos:main Aug 3, 2023
22 of 23 checks passed
@micahsnyder
Copy link
Contributor

Thank you @rzvncj !!!

@micahsnyder
Copy link
Contributor

Looking at the code, I've seen this sort of thing happening a bit:

if (attempt_str) {
    free(attempt_str);
}

But free(NULL) should be perfectly safe (according to 7.20.3.2 of ISO-IEC 9899), so it seems like we could simplify those. Do we need to work with non-compliant compilers?

Forgot to reply to this. It's fine to skip NULL checks for free(). I think we often do it habitually, but it doesn't really matter. Though there are some functions for cleaning up structures where you must do a NULL-check first. In that sense, it may be safer to be in the habit of doing NULL-checks. 🤷

@rzvncj
Copy link
Contributor Author

rzvncj commented Aug 3, 2023

Thank you @rzvncj !!!

Happy to help! 👍

@rzvncj rzvncj deleted the issue433 branch August 4, 2023 11:31
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

Successfully merging this pull request may close these issues.

clamonacc --wait reporting "cli_strdup(): s == NULL"
2 participants