Skip to content

feat: add zALLMATCHSCAN#5

Open
McPatate wants to merge 21 commits intoLevitatingOrange:mainfrom
huggingface:feat/add_allmatch
Open

feat: add zALLMATCHSCAN#5
McPatate wants to merge 21 commits intoLevitatingOrange:mainfrom
huggingface:feat/add_allmatch

Conversation

@McPatate
Copy link
Copy Markdown
Collaborator

@McPatate McPatate commented Jan 17, 2023

  • Add zALLMATCHSCAN command.
  • Refactored the decoder as ClamAV can output multiple \0 terminated lines.
  • Also added a check that clamd is running on cargo test to improve dev exp.

Note that I will rebase #4 once it's merged so that this PR contains only the last commit 740e442

@McPatate
Copy link
Copy Markdown
Collaborator Author

I've realised that there must be some issue in the parsing of the stream. After a few commands when .keep_alive(true), the client goes into error. Looking into it

@McPatate McPatate force-pushed the feat/add_allmatch branch from 7bd3852 to ff4cae9 Compare June 22, 2023 14:44
@McPatate
Copy link
Copy Markdown
Collaborator Author

Very strange, tests pass on my machine. Could you check if tests also work for you @LevitatingOrange ?

@LevitatingOrange
Copy link
Copy Markdown
Owner

Very strange, tests pass on my machine. Could you check if tests also work for you @LevitatingOrange ?

Yeah weird, they pass on my machine too.

@McPatate
Copy link
Copy Markdown
Collaborator Author

McPatate commented Jul 18, 2023

@LevitatingOrange do you have some time to review ? It works nicely in production.

I was wondering, do we want to deduplicate when ClamAV reports the same signature multiple times?

EDIT: regarding ^, now that I think of it, it may be useless/annoying to do once we switch to Vec<ScanResult>. If it were only strings, we could easily just do HashSet<String>, but with ScanResults I wonder if this makes sense. We should let the user do his own thing if he wants to dedup signatures, wdyt?

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.

2 participants