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

Next/20210823/v4 #6304

Merged
merged 24 commits into from
Aug 23, 2021
Merged

Next/20210823/v4 #6304

merged 24 commits into from
Aug 23, 2021

Conversation

victorjulien
Copy link
Member

jufajardini and others added 24 commits August 10, 2021 15:57
All cases of our transmute can be replaced with more idiomatic
solutions and do no require the power of transmute.

When returning an object to C for life-time management, use
Box::into_raw to convert the boxed object to pointer and use
Box::from_raw to convert back.

For cases where we're just returning a pointer to Rust managed
data, use a cast.
Based on the Rust clippy lint that recommends that any public
function that dereferences a raw pointer, mark all FFI functions
that reference raw pointers with build_slice and cast_pointer
as unsafe.

This commits starts by removing the unsafe wrapper inside
the build_slice and cast_pointer macros then marks all
functions that use these macros as unsafe.

Then fix all not_unsafe_ptr_arg_deref warnings from clippy.

Fixes clippy lint:
https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref
These are needless borrows (references) as the item is already
a reference.
This lint checks for a closure where a function can be directly
supplied.  Runtime performance is unchanged, but this makes
less work for the compiler.
Calling a function in unwrap_or causes that function to always
be called even when not needed. Instead use unwrap_or_else with
a closure which will only be called when needed.
In these simple cases to_string() is recommended and likely
performs better as the formatter is not called.
These add complexity and may not be optimized out by the compiler.
This is a readability fix, as on first look they almost look
like a Rust tuple.
This is code that is not needed and is a bit confusing to see.
When defaulting checked_mul to u64::max, Rust has a method
that does the same thing called saturating_mul.
- ref is discouraged for top level variables
- the other borrow is not required
This is the preffered style and easier to understand the meaning
of the code.
Using `if let` expressions in these cases is better expressed
by the map method, and considered idiomatic Rust for this usage.
Suppress all remaining clippy lints that we trip. This can be
fixed on a per-lint basis.
@victorjulien victorjulien requested review from jasonish and a team as code owners August 23, 2021 09:51
@codecov
Copy link

codecov bot commented Aug 23, 2021

Codecov Report

Merging #6304 (cf21694) into master (9d24a53) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #6304   +/-   ##
=======================================
  Coverage   76.94%   76.94%           
=======================================
  Files         611      611           
  Lines      185941   185941           
=======================================
+ Hits       143064   143081   +17     
+ Misses      42877    42860   -17     
Flag Coverage Δ
fuzzcorpus 52.86% <100.00%> (+0.11%) ⬆️
suricata-verify 51.10% <100.00%> (-0.06%) ⬇️
unittests 63.12% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@victorjulien
Copy link
Member Author

CIFuzz compile failure:

2021-08-23T10:04:55.7283753Z   CXXLD    fuzz_applayerprotodetectgetproto
2021-08-23T10:05:00.9688782Z /usr/bin/ld: __sancov_cntrs has both ordered [`__sancov_cntrs[ROHashInitFinalize]' in libsuricata_c.a(util-rohash.o)] and unordered [`__sancov_cntrs[_ZN102_$LT$core..iter..adapters..map..Map$LT$I$C$F$GT$$u20$as$u20$core..iter..traits..iterator..Iterator$GT$4fold17h0cf2d506a4e568d1E]' in ../rust/target/x86_64-unknown-linux-gnu/release/libsuricata_rust.a(suricata_rust-6f68cb102e8bc119.suricata_rust.7485b842-cgu.0.rcgu.o)] sections
2021-08-23T10:05:00.9693367Z /usr/bin/ld: final link failed: Bad value
2021-08-23T10:05:00.9846248Z /usr/bin/ld: __sancov_cntrs has both ordered [`__sancov_cntrs[ROHashInitFinalize]' in libsuricata_c.a(util-rohash.o)] and unordered [`__sancov_cntrs[_ZN102_$LT$core..iter..adapters..map..Map$LT$I$C$F$GT$$u20$as$u20$core..iter..traits..iterator..Iterator$GT$4fold17h0cf2d506a4e568d1E]' in ../rust/target/x86_64-unknown-linux-gnu/release/libsuricata_rust.a(suricata_rust-6f68cb102e8bc119.suricata_rust.7485b842-cgu.0.rcgu.o)] sections
2021-08-23T10:05:00.9848719Z /usr/bin/ld: final link failed: Bad value
2021-08-23T10:05:00.9890460Z clang-12: error: linker command failed with exit code 1 (use -v to see invocation)
2021-08-23T10:05:00.9928261Z make[2]: *** [fuzz_applayerprotodetectgetproto] Error 1
2021-08-23T10:05:00.9929310Z make[2]: *** Waiting for unfinished jobs....
2021-08-23T10:05:00.9930580Z Makefile:2293: recipe for target 'fuzz_applayerprotodetectgetproto' failed
2021-08-23T10:05:01.0023435Z clang-12: error: linker command failed with exit code 1 (use -v to see invocation)
2021-08-23T10:05:01.0051389Z make[2]: *** [suricata] Error 1
2021-08-23T10:05:01.0052406Z Makefile:2335: recipe for target 'suricata' failed
2021-08-23T10:05:01.0054910Z make[2]: Leaving directory '/src/suricata/src'
2021-08-23T10:05:01.0062286Z Makefile:2168: recipe for target 'all' failed
2021-08-23T10:05:01.0062863Z make[1]: *** [all] Error 2
2021-08-23T10:05:01.0068346Z make[1]: Leaving directory '/src/suricata/src'
2021-08-23T10:05:01.0075048Z Makefile:494: recipe for target 'all-recursive' failed
2021-08-23T10:05:01.0075861Z make: *** [all-recursive] Error 1
2021-08-23T10:05:01.6802704Z 2021-08-23 10:05:01,679 - root - ERROR - Building fuzzers failed.
2021-08-23T10:05:01.6804289Z 2021-08-23 10:05:01,679 - root - ERROR - Error building fuzzers for (commit: a0c7f35f00359a0540b8c9c5d6f46306d65f339b, pr_ref: refs/pull/6304/merge).

@catenacyber
Copy link
Contributor

CIFuzz failure is due to oss-fuzz build failure due to a regression in rust nightly rust-lang/rust#53945

@catenacyber
Copy link
Contributor

I meant rust-lang/rust#88258

@victorjulien victorjulien deleted the next/20210823/v4 branch August 31, 2021 05:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants