I ran a build using clang's -Weverything to activate all warnings (setting CFLAG in the top-level Makefile and removing all other warning-related flags, especially -Werror). With the system clang on my mac, master at 4d04226c2ec7e7f69f6234def63631648e35e828 with the patch from #3984 gives the following warnings:
94 [-Wcast-align] 145 [-Wconversion] 1308 [-Wdocumentation] 84 [-Wextended-offsetof] 6 [-Wformat-nonliteral] 331 [-Wlanguage-extension-token] 453 [-Wmissing-field-initializers] 1 [-Wmissing-noreturn] 237 [-Wmissing-variable-declarations] 7297 [-Wpadded] 758 [-Wshorten-64-to-32] 1647 [-Wsign-conversion] 17 [-Wswitch-enum] 1 [-Wunreachable-code-break] 5 [-Wunreachable-code] 9422 [-Wunused-macros] 661 [-Wunused-parameter] Some of these warnings are not actually useful diagnostics for our use case and should be ignored (-Wpadded, for example, and -Wlanguage-extension-token complaining about the use of inline assembly), so we should not attempt to go for 0 warnings across the board. But some of them are helpful to point to places where the code could be improved. Applying the attached patch series (from git format-patch --stdout) brings things down to 94 [-Wcast-align] 145 [-Wconversion] 84 [-Wextended-offsetof] 4 [-Wformat-nonliteral] 331 [-Wlanguage-extension-token] 453 [-Wmissing-field-initializers] 1 [-Wmissing-noreturn] 82 [-Wmissing-variable-declarations] 7297 [-Wpadded] 759 [-Wshorten-64-to-32] 1645 [-Wsign-conversion] 17 [-Wswitch-enum] 3 [-Wunreachable-code] 9422 [-Wunused-macros] 661 [-Wunused-parameter] Or, in diff form 3d2 < 1308 [-Wdocumentation] 5c4 < 6 [-Wformat-nonliteral] --- > 4 [-Wformat-nonliteral] 9c8 < 237 [-Wmissing-variable-declarations] --- > 82 [-Wmissing-variable-declarations] 11,12c10,11 < 758 [-Wshorten-64-to-32] < 1647 [-Wsign-conversion] --- > 759 [-Wshorten-64-to-32] > 1645 [-Wsign-conversion] 14,15c13 < 1 [-Wunreachable-code-break] < 5 [-Wunreachable-code] --- > 3 [-Wunreachable-code] The attached patches can also be found at https://github.com/kaduk/openssl/commits/warning-cleanup . I did not make a pull request yet since the commits are broken out for ease of describing the included changes; they could very well be consolidated into a smaller number of commits into the main repo. I was a little uncertain about marking the secure_mem variable in sec_mem.c as static, since it seemed like the intent was to export that information; on the other hand, the variable was not declared in any header, which seems to make it not part of any public API. From the comments on pull request 381, it sounds like Rich wants to add an accessor function, in which case making the actual variable static should be fine. -Ben Kaduk
warning-cleanup.patch
Description: Binary data
_______________________________________________ openssl-bugs-mod mailing list [email protected] https://mta.openssl.org/mailman/listinfo/openssl-bugs-mod
_______________________________________________ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
