| Message ID | 1461097517-21626-1-git-send-email-thomas.petazzoni@free-electrons.com |
|---|---|
| State | Deferred |
| Delegated to: | Stephen Warren |
| Headers | show |
On 04/19/2016 02:25 PM, Thomas Petazzoni wrote: > The current build system adds "-isystem /usr/include/$(CRYPTOLIB)" to > AM_CPPFLAGS, but this is wrong because cryptopp might not be installed > in this location. Instead, the build system should simply include > <cryptopp/...> or <crypto++/...> and rely on the compiler include > path. > > The tricky part is that it can be <cryptopp/...> or <crypto++/...>. To > solve this, we use a solution similar to the one used in > https://github.com/bingmann/crypto-speedtest/blob/master/m4/cryptopp.m4 > and > https://github.com/bingmann/crypto-speedtest/blob/master/src/speedtest_cryptopp.cpp: > the configure script fills in a variable called > CRYPTOLIB_HEADER_PREFIX, and we use that in the C++ code to include > the right header file. > > It is worth mentioning that doing #include > <CRYPTOLIB_HEADER_PREFIX/foobar.h> doesn't work, and we have to use an > intermediate #define'd constant to overcome this C preprocessor > limitation. I think this looks conceptually OK. CC += Allen to double-check since he wrote the original cryptopp autoconf support. Could you please re-spin this on top of the latest git commits? There's a new file rsa-pss.cpp that needs to be updated too. Without that, this patch causes compile failures. > diff --git a/src/aes-cmac.cpp b/src/aes-cmac.cpp > -#include "cryptlib.h" > -using CryptoPP::Exception; > +#define CRYPTOPP_INCLUDE_CRYPTLIB <CRYPTOLIB_HEADER_PREFIX/cryptlib.h> > +#define CRYPTOPP_INCLUDE_CMAC <CRYPTOLIB_HEADER_PREFIX/cmac.h> > +#define CRYPTOPP_INCLUDE_AES <CRYPTOLIB_HEADER_PREFIX/aes.h> > +#define CRYPTOPP_INCLUDE_HEX <CRYPTOLIB_HEADER_PREFIX/hex.h> > +#define CRYPTOPP_INCLUDE_FILTERS <CRYPTOLIB_HEADER_PREFIX/filters.h> > +#define CRYPTOPP_INCLUDE_SECBLOCK <CRYPTOLIB_HEADER_PREFIX/secblock.h> > > -#include "cmac.h" > -using CryptoPP::CMAC; > +#include CRYPTOPP_INCLUDE_CRYPTLIB > +#include CRYPTOPP_INCLUDE_CMAC > +#include CRYPTOPP_INCLUDE_AES > +#include CRYPTOPP_INCLUDE_HEX > +#include CRYPTOPP_INCLUDE_FILTERS > +#include CRYPTOPP_INCLUDE_SECBLOCK It'd be nice to avoid re-ordering everything; just edit the existing lines in place. That'd make the diff smaller, and make it more obvious that the only thing changing is the include filenames and not the using statements. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 19, 2016 at 04:32:46PM -0600, Stephen Warren wrote: > On 04/19/2016 02:25 PM, Thomas Petazzoni wrote: > >The current build system adds "-isystem /usr/include/$(CRYPTOLIB)" to > >AM_CPPFLAGS, but this is wrong because cryptopp might not be installed > >in this location. Instead, the build system should simply include > ><cryptopp/...> or <crypto++/...> and rely on the compiler include > >path. > > > >The tricky part is that it can be <cryptopp/...> or <crypto++/...>. To > >solve this, we use a solution similar to the one used in > >https://github.com/bingmann/crypto-speedtest/blob/master/m4/cryptopp.m4 > >and > >https://github.com/bingmann/crypto-speedtest/blob/master/src/speedtest_cryptopp.cpp: > >the configure script fills in a variable called > >CRYPTOLIB_HEADER_PREFIX, and we use that in the C++ code to include > >the right header file. > > > >It is worth mentioning that doing #include > ><CRYPTOLIB_HEADER_PREFIX/foobar.h> doesn't work, and we have to use an > >intermediate #define'd constant to overcome this C preprocessor > >limitation. > > I think this looks conceptually OK. CC += Allen to double-check > since he wrote the original cryptopp autoconf support. Won't the same problem with the naming happen when we link as well? (The "-lcryptopp") ? Can pkg-config help here? -Allen -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/configure.ac b/configure.ac index 943654f..620e158 100644 --- a/configure.ac +++ b/configure.ac @@ -33,6 +33,7 @@ AC_LINK_IFELSE( [AC_MSG_ERROR([libcrypto++/libcryptopp is not installed.])])] ) AC_SUBST(CRYPTOLIB) +AC_DEFINE_UNQUOTED([CRYPTOLIB_HEADER_PREFIX], [$CRYPTOLIB], [Location of cryptolib header]) LDFLAGS=$SAVED_LDFLAGS AC_LANG(C) diff --git a/src/Makefile.am b/src/Makefile.am index 3dad0e6..35a606f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1,5 +1,5 @@ AM_CFLAGS = -Wall -std=c99 -AM_CPPFLAGS = -isystem /usr/include/$(CRYPTOLIB) $(LIBUSB_CFLAGS) +AM_CPPFLAGS = $(LIBUSB_CFLAGS) bin_PROGRAMS = tegrarcm tegrarcm_SOURCES = \ diff --git a/src/aes-cmac.cpp b/src/aes-cmac.cpp index 24c89f8..da8be5a 100644 --- a/src/aes-cmac.cpp +++ b/src/aes-cmac.cpp @@ -26,6 +26,9 @@ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ + +#include "config.h" + #include <iostream> using std::cout; using std::cerr; @@ -40,26 +43,29 @@ using std::string; #include <cstdlib> using std::exit; -#include "cryptlib.h" -using CryptoPP::Exception; +#define CRYPTOPP_INCLUDE_CRYPTLIB <CRYPTOLIB_HEADER_PREFIX/cryptlib.h> +#define CRYPTOPP_INCLUDE_CMAC <CRYPTOLIB_HEADER_PREFIX/cmac.h> +#define CRYPTOPP_INCLUDE_AES <CRYPTOLIB_HEADER_PREFIX/aes.h> +#define CRYPTOPP_INCLUDE_HEX <CRYPTOLIB_HEADER_PREFIX/hex.h> +#define CRYPTOPP_INCLUDE_FILTERS <CRYPTOLIB_HEADER_PREFIX/filters.h> +#define CRYPTOPP_INCLUDE_SECBLOCK <CRYPTOLIB_HEADER_PREFIX/secblock.h> -#include "cmac.h" -using CryptoPP::CMAC; +#include CRYPTOPP_INCLUDE_CRYPTLIB +#include CRYPTOPP_INCLUDE_CMAC +#include CRYPTOPP_INCLUDE_AES +#include CRYPTOPP_INCLUDE_HEX +#include CRYPTOPP_INCLUDE_FILTERS +#include CRYPTOPP_INCLUDE_SECBLOCK -#include "aes.h" +using CryptoPP::Exception; +using CryptoPP::CMAC; using CryptoPP::AES; - -#include "hex.h" using CryptoPP::HexEncoder; using CryptoPP::HexDecoder; - -#include "filters.h" using CryptoPP::StringSink; using CryptoPP::StringSource; using CryptoPP::HashFilter; using CryptoPP::HashVerificationFilter; - -#include "secblock.h" using CryptoPP::SecByteBlock; extern "C" int cmac_hash(const unsigned char *msg, int len, unsigned char *cmac_buf)
The current build system adds "-isystem /usr/include/$(CRYPTOLIB)" to AM_CPPFLAGS, but this is wrong because cryptopp might not be installed in this location. Instead, the build system should simply include <cryptopp/...> or <crypto++/...> and rely on the compiler include path. The tricky part is that it can be <cryptopp/...> or <crypto++/...>. To solve this, we use a solution similar to the one used in https://github.com/bingmann/crypto-speedtest/blob/master/m4/cryptopp.m4 and https://github.com/bingmann/crypto-speedtest/blob/master/src/speedtest_cryptopp.cpp: the configure script fills in a variable called CRYPTOLIB_HEADER_PREFIX, and we use that in the C++ code to include the right header file. It is worth mentioning that doing #include <CRYPTOLIB_HEADER_PREFIX/foobar.h> doesn't work, and we have to use an intermediate #define'd constant to overcome this C preprocessor limitation. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- configure.ac | 1 + src/Makefile.am | 2 +- src/aes-cmac.cpp | 28 +++++++++++++++++----------- 3 files changed, 19 insertions(+), 12 deletions(-)