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

Rust: generic files definition #5513

Closed
wants to merge 1 commit into from

Conversation

jufajardini
Copy link
Contributor

Issue: Optimization 3825

  • filecontainer: add Files structure, to replace/unify SMBFiles
    and NFSFiles
  • smb/files: delete SMBFiles implementation
  • smb/smb: replace SMBFiles with Files
  • nfs/nfs: delete NFSFiles implementation, replace its former
    declarations with Files' ones

Make sure these boxes are signed before submitting your Pull Request -- thank you.

Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/3825

Link to previous PR:
#5511

Describe changes:

  • filecontainer: add Files structure, to replace/unify SMBFiles and NFSFiles
  • smb/files: delete SMBFiles implementation
  • smb/smb: replace SMBFiles with Files
  • nfs/nfs: delete NFSFiles implementation, replace its former declarations with Files' ones
  • squash commits to keep things clean

PRScript output (if applicable):

#suricata-verify-pr:
#suricata-verify-repo:
#suricata-verify-branch:
#suricata-update-pr:
#suricata-update-repo:
#suricata-update-branch:
#libhtp-pr:
#libhtp-repo:
#libhtp-branch:

Issue: Optimization 3825
- filecontainer: add Files structure, to replace/unify SMBFiles
 and NFSFiles
- smb/files: delete SMBFiles implementation
- smb/smb: replace SMBFiles with Files
- nfs/nfs: delete NFSFiles implementation, replace its former
 declarations with Files' ones
@jufajardini
Copy link
Contributor Author

Hi, have checked the details for the failed checks on appveyor, but I'm not quite sure what's wrong on my side. Can you please help me understand what do I have to do here, or if there's something else in my code that may be causing this, and I wasn't able to realise?

Thanks in advance! :)

@inashivb
Copy link
Member

About appveyor: Doesn't seem like an issue on your end. :)

@jasonish
Copy link
Member

Appveyor seems to be having issues today.

Copy link
Contributor

@catenacyber catenacyber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also HTTP2 in addition to SMB and NFS which should be replaced

@jufajardini
Copy link
Contributor Author

There is also HTTP2 in addition to SMB and NFS which should be replaced

Yeah, I saw that in the original discussion, but since it was not listed in this issue, and since I'm still new to the project, I need some approvals before tackling that.

Can it be done right now, in this very branch/issue?

@catenacyber
Copy link
Contributor

Yes it can be done in this issue.
But you can create a new branch optimization-3825-v3

@jufajardini
Copy link
Contributor Author

ok, thanks! will try to do another PR tomorrow evening, then.

@jufajardini jufajardini mentioned this pull request Nov 4, 2020
3 tasks
@jufajardini
Copy link
Contributor Author

Replaced by: #5531

to implement changes requested by @catenacyber .

@jufajardini jufajardini closed this Nov 4, 2020
@jufajardini jufajardini deleted the optimization-3825-v2 branch December 11, 2020 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants