-
-
Notifications
You must be signed in to change notification settings - Fork 196
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
Add list parsing feature for gravity #1559
Conversation
Signed-off-by: DL6ER <[email protected]>
Signed-off-by: DL6ER <[email protected]>
…ine starts in "|") Signed-off-by: DL6ER <[email protected]>
Signed-off-by: DL6ER <[email protected]>
…ion and transform FQDN to domains. They are equivalent in this context but now they are not considered invalid any longer Signed-off-by: DL6ER <[email protected]>
Signed-off-by: DL6ER <[email protected]>
Signed-off-by: DL6ER <[email protected]>
Signed-off-by: DL6ER <[email protected]>
Signed-off-by: DL6ER <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested inside a container and it is working as expected.
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Signed-off-by: DL6ER <[email protected]>
Conflicts have been resolved. |
while(isspace(line[0])) | ||
line++; | ||
|
||
// Skip comments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is some duplicate code L166-186. This is still part of gravity.sh
gravity_ParseFileIntoDomains()
Does not hurt to keep it, just wanted to report.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it is duplicated, we should remove it from FTL for minimal speed gain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, rethinking this, we can also remove gravity_ParseFileIntoDomains()
from gravity.sh
and use the code here that should be a lot faster.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then this would be something for pi-hole/pi-hole#5275
I think I approved too soon. I noticed the output here is saying
Comparing to EDIT: The original function excludes false positives from non-domains count. |
@rdwebdesign This is expected and correct behavior. Each non-domain is counted but then only unique ones are shown. So, if there are 13 entries with
I'm always open to improving wording in case this isn't clear right now. |
Maybe simply changing the message to "Sample of non-domain entries (duplicates removed)" |
What does this implement/fix?
This PR implements a list parsing feature into FTL that is intended to be used with gravity. Its main advantage is a stream-lined approach directly writing everything directly into the new database during analysis. There is no detour over huge allocated memory blocks or temporary files.
Related issue or feature (if applicable): N/A
Pull request in docs with documentation (if applicable): N/A
By submitting this pull request, I confirm the following:
git rebase
)Checklist:
developmental
branch.