Suricata

Hello.
I would like to claim this issue as my first contribution with outreachy

Hi Gabriel,

thanks for your interest in contributing to our project!
Feel free to assign the task to yourself, you already have the proper roles for that ^^

Hi again, I have a few questions about this ticket, I'm going to ask all of them in this post.
I hope that isn't a bad practice, but if It is please let me know.

1)Correct if I'm wrong but the goal of this issue is remove equal block of code that appears
both in If Else cases. I found an case that I didn't quite understood. The code is on the file
suricata/rust/src/core.rs, line 68 to 79. I will copy It here, but maybe look at It in the file would
be better

        if d & (DIR_TOSERVER | DIR_TOCLIENT) == (DIR_TOSERVER | DIR_TOCLIENT) {
            debug_validate_fail!("Both directions are set");
            Direction::ToServer
        } else if d & DIR_TOSERVER != 0 { 
            Direction::ToServer
        } else if d & DIR_TOCLIENT != 0 { 
            Direction::ToClient
        } else {
            debug_validate_fail!("Unknown direction!!");
            Direction::ToServer
        }

I'm having some difficult on how to remove the repetition of the Direction::ToServer statement in lines 70 and 72.

2)How do I know if the tests have coverage for the Code I have Modify ?

3)If they don't I have to write tests for that bits of codes? if there a page show whats the pattern for writing those questions ?

I'm sorry if some of the questions are not well written, English is not my main language and I'm still getting the hang of it.
and sorry for the long post, It's my first time working in a project this, so I'm in the process of getting used to it

Hi Gabriel,

Answering your questions:

1) From the error that `cargo clippy` gives, my understanding is that you could probably remove the first `else if` statement,
since the first `if` statement already covers that case. Does that make sense?

error: this `if` has identical blocks
  --> src/core.rs:68:79
   |
68 |           if d & (DIR_TOSERVER | DIR_TOCLIENT) == (DIR_TOSERVER | DIR_TOCLIENT) {
   |  _______________________________________________________________________________^
69 | |             debug_validate_fail!("Both directions are set");
70 | |             Direction::ToServer
71 | |         } else if d & DIR_TOSERVER != 0 {
   | |_________^
   |
   = note: `#[deny(clippy::if_same_then_else)]` on by default
note: same as this
  --> src/core.rs:71:41
   |
71 |           } else if d & DIR_TOSERVER != 0 {
   |  _________________________________________^
72 | |             Direction::ToServer
73 | |         } else if d & DIR_TOCLIENT != 0 {
   | |_________^
   = help: for further information visit http

2) Code coverage isn't the goal of this task, but: whenever someone submits a pull request, we have a bot that will generate a code coverage report. You can check it, and see what was the impact of your change ;)

3) You don't have to write any tests for these changes. When a PR is submitted, our integration checks will run on your code, and these will trigger a run of all of our unittests and suricata-verify tests. Since you are changing existing code that already works, if any of your changes broke anything, those would likely catch that :)
If you want to read more about testing Suricata in general, you may check our docs: https://suricata.readthedocs.io/en/latest/devguide/codebase/testing.html

Lastly: don't worry about your English: you got your message across, and everyone makes mistakes, even in their mother tongue ;)
But if that would help you feel more confident, you could take a look at grammarly. I use it often, and it offers precious tips.

One final comment:
when you have a question that is directly related to a portion of code, another thing you can do is submit a draft pull request, and add a comment on the part related to your question. This makes it easier for us to visualize and understand.

In that case, after submitting the draft PR, you may reach out to us and mention that you're stuck, and share a link to the PR.

A long answer for a long post, hope it helps!