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

Remove dead code #531

Merged
merged 17 commits into from
Aug 9, 2021
Merged

Remove dead code #531

merged 17 commits into from
Aug 9, 2021

Conversation

bifurcation
Copy link
Contributor

As I was looking into migrating things to Rust, I noticed there are a few chunks of around. In addition to the ones I noticed in the SHA-1 code, I scanned for #if 0 blocks and used cflow -r to find unused functions.

*/
srtp_err_status_t srtp_replace_auth_type(const srtp_auth_type_t *ct,
srtp_auth_type_id_t id);

Choose a reason for hiding this comment

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

The replace functions are API, not dead code - they're to allow a third-party crypto implementation to be injected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a little surprising to me; I didn't realize the crypto/include headers were considered public API.

Choose a reason for hiding this comment

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

Just crypto/include/cipher.h, crypto/include/auth.h, and crypto/include/crypto_types.h are public - see the install targets in Makefile.in or CMakeLists.txt.

@@ -302,19 +277,14 @@ void v128_left_shift(v128_t *x, int shift)

int bitvector_get_bit(const bitvector_t *v, int bit_index)
{
return _bitvector_get_bit(v, bit_index);
return /bitvector_get_bit(v, bit_index);

Choose a reason for hiding this comment

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

??? Does this compile?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does, but only because it's within #ifndef DATATYPES_USE_MACROS, and that doesn't appear to be the case for either my local build (macOS, CMake, default config) or any of the CI builds. Maybe we should remove that option?

Choose a reason for hiding this comment

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

DATATYPES_USE_MACROS is defined unconditionally in datatypes.h, so yeah, removing its #ifndef conditions is probably a good idea.

@pabuhler pabuhler added this to the Version 2.4.0 milestone Apr 7, 2021
@pabuhler
Copy link
Member

pabuhler commented Apr 7, 2021

@bifurcation could you fix the clang format errors

@bifurcation
Copy link
Contributor Author

@pabuhler Looks like all builds are passing now except for the broken mips/meson build.

@pabuhler pabuhler merged commit 4f4d141 into cisco:master Aug 9, 2021
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.

3 participants