Repository navigation
fix: Allow inline comments in clamd.conf - #1308
Conversation
|
Thanks for the PR! I will try to review this after we have things lined up to publish 1.4.0, or after 1.4.0 ships. |
ef7d483 to
8e9d15f
Compare
val-ms
left a comment
There was a problem hiding this comment.
I rebased this PR to re-run the tests.
I found just minor clang-format issues.
In manual testing, I found that for string-based options (vs bool, size, or number args) -- spaces before a # end up getting included in the option value.
For example, I tried this for freshclam.conf:
DatabaseMirror http://localhost:8000 # database.clamav.net
The result was that it tried to download daily.cvd from:
http://localhost:8000 /daily.cvd (with a space in the middle).
I would have to remove the space between 8000 and # to make it work. Like this:
DatabaseMirror http://localhost:8000# database.clamav.net
But this doesn't look very natural and is bound to get complaints from users that try this feature.
I also had trouble with getting a bool to work. I tried this in freshclam.conf, but it didn't enable verbose mode:
LogVerbose yes #no
|
Yeah, the Tried to see how I could implement a test for this fix, but I'm not sure if its possible to printout the options even in debug mode, as its managed in Other caveatIf someone has |
… files The config parser will now treat a '#' character in a config file as an inline comment, allowing users to write configs like this: ```freshclam.conf DatabaseMirror http://localhost:8000 # My private server ```
E.g. this will also be valid: ```freshclam.conf DatabaseMirror http://localhost:8000<tab># My private server. ``` Also: - Fix compile warning regarding unnecessarily const string variable. - Add parenthesis to resolve inconsistency between clang-format and vscode auto-format. - Add a macro defining the max config line length.
971f3e7 to
7bfe63f
Compare
val-ms
left a comment
There was a problem hiding this comment.
Thanks for fixing the clang-format issue and also the trailing spaces issue. I tried a variety of variations of trailing spaces for options with strings, booleans, numbers. As far as I can tell it works nicely.
I am seeing a warning with this change when building:
/home/micah/dev/clamav/clamav-1/common/optparser.c: In function ‘optparse’:
/home/micah/dev/clamav/clamav-1/common/optparser.c:1195:35: warning: passing argument 1 of ‘strtok’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
1195 | arg = strtok(arg, "#");
| ^~~
In file included from /home/micah/dev/clamav/clamav-1/common/optparser.c:36:
/usr/include/string.h:336:14: note: expected ‘char * __restrict__’ but argument is of type ‘const char *’
336 | extern char *strtok (char *__restrict __s, const char *__restrict __delim)
| ^~~~~~
/home/micah/dev/clamav/clamav-1/common/optparser.c:1196:26: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
1196 | trim_comment = arg + strlen(arg) - 1;
| ^
I took the liberty of fixing this and extending your change to also trim trailing tabs. I tidied the commits and rebased them with main so we've got all the latest commits in the branch. Pending a good run through GitHub Actions and Jenkins, I'm happy with this and will merge it.
|
Thanks again for your work on this! |
This PR attempts to resolve the issue mentioned in #1175
#and if it does, try to get the content on its left side.Thoughts:
strtokis the best choice.cli_regexecreturns the made matches (most likely in the 4th argument, but it seems its used mostly withNULLthrough out the project).