headers
Stephen torri | 3 Nov 2003 06:18

[Fwd: Re: Compiling crypto++ with -Wall -Werror]

I am trying to create a text editor program that can be used to encrypt
and decrypt files. In order to ensure that there are no security holes
in the code I link in I am compiling crypto++ with the gcc options of
'-g -Wall -Werror'.=20

In doing so I am noticing a few things missed with the old compiler
options of just '-g'. Is there a preferred format for sending in patches
to fix these warnings?

Here are the errors I get with crypto++ (v. CVS) and gcc (v. 3.2.3):

g++ -g -Wall -Werror -pipe -c 3way.cpp
cc1plus: warnings being treated as errors
In file included from seckey.h:10,
                 from 3way.h:7,
                 from 3way.cpp:5:
simple.h: In member function `unsigned int
   CryptoPP::InputRejecting<T>::Put2(const byte*, unsigned int, int,
bool)':
simple.h:94: warning: no return statement in function returning non-void
simple.h: In member function `bool
   CryptoPP::InputRejecting<T>::IsolatedMessageSeriesEnd(bool)':
simple.h:96: warning: no return statement in function returning non-void
simple.h: In member function `unsigned int
   CryptoPP::InputRejecting<T>::ChannelPut2(const std::string&, const
byte*,
   unsigned int, int, bool)':
simple.h:99: warning: no return statement in function returning non-void
simple.h: In member function `bool
   CryptoPP::InputRejecting<T>::ChannelMessageSeriesEnd(const
(Continue reading)

Permalink | Reply | 0 comments |
headers
Stephen torri | 3 Nov 2003 06:28

Compiling crypto++ with -Wall -Werror

I am trying to create a text editor program that can be used to encrypt
and decrypt files. In order to ensure that there are no security holes
in the code I link in I am compiling crypto++ with the gcc options of
'-g -Wall -Werror'. 

In doing so I am noticing a few things missed with the old compiler
options of just '-g'. Is there a preferred format for sending in patches
to fix these warnings?

Here are the errors I get with crypto++ (v. CVS) and gcc (v. 3.2.3):

g++ -g -Wall -Werror -pipe -c 3way.cpp
cc1plus: warnings being treated as errors
In file included from seckey.h:10,
                 from 3way.h:7,
                 from 3way.cpp:5:
simple.h: In member function `unsigned int
   CryptoPP::InputRejecting<T>::Put2(const byte*, unsigned int, int,
bool)':
simple.h:94: warning: no return statement in function returning non-void
simple.h: In member function `bool
   CryptoPP::InputRejecting<T>::IsolatedMessageSeriesEnd(bool)':
simple.h:96: warning: no return statement in function returning non-void
simple.h: In member function `unsigned int
   CryptoPP::InputRejecting<T>::ChannelPut2(const std::string&, const
byte*,
   unsigned int, int, bool)':
simple.h:99: warning: no return statement in function returning non-void
simple.h: In member function `bool
   CryptoPP::InputRejecting<T>::ChannelMessageSeriesEnd(const
(Continue reading)

Permalink | Reply | 0 comments |
headers
Stephen torri | 3 Nov 2003 06:50

[PATCH] algparam.h - Unused assert

The assert statement is reported as being unused. This was test both
with the compiler flags set to '-g' and '-O2'.

Stephen

Index: algparam.h
===================================================================
RCS file: /cvsroot/cryptopp/c5/algparam.h,v
retrieving revision 1.6
diff -U2 -r1.6 algparam.h
--- algparam.h	18 Jul 2003 04:35:30 -0000	1.6
+++ algparam.h	3 Nov 2003 05:21:05 -0000
 <at>  <at>  -24,6 +24,14  <at>  <at> 
 	template <class T> ConstByteArrayParameter(const T &string, bool
deepCopy = false)
 	{
-		CRYPTOPP_COMPILE_ASSERT(sizeof(string[0])==1);
-		Assign((const byte *)string.data(), string.size(), deepCopy);
+	  /* FIXME: Unused -
+	     CRYPTOPP_COMPILE_ASSERT(sizeof(string[0])==1);
+
+	     SUGGESTION: Consider throwing an exception here to allow the
+	     program to exit gracefully and be more informative. Without
+	     an exception a programmer needs to use a stack trace via a 
+	     core dump to figure out why crypto++ or their application
+	     caused this condition to be true
+	  */
+	  Assign((const byte *)string.data(), string.size(), deepCopy);
 	}
(Continue reading)

Permalink | Reply | 0 comments |
headers
Stephen torri | 3 Nov 2003 06:58

[PATCH] basecode.cpp - Unused code - Problem with FILTER_OUTPUT

The macros used to weave in the case statements are not working. The gcc
compiler is not expanding the macros. Here is the patch I used to clear
the warnings. Its not a fix which that should be evident. What I can
send upon request is the preprocessor output. The file I have is 60K in
size and I am not sure if the maillist server will allow that as an
attachment. I certainly do not want to inline it.

Index: basecode.cpp
===================================================================
RCS file: /cvsroot/cryptopp/c5/basecode.cpp,v
retrieving revision 1.8
diff -U2 -r1.8 basecode.cpp
--- basecode.cpp	18 Jul 2003 04:35:30 -0000	1.8
+++ basecode.cpp	3 Nov 2003 05:41:02 -0000
 <at>  <at>  -79,6 +79,13  <at>  <at> 
 				m_outBuf[i] = m_alphabet[m_outBuf[i]];
 			}
-			FILTER_OUTPUT(1, m_outBuf, m_outputBlockSize, 0);
+			/* FIXME: statement has no effect
+			   
+			QUESTION: are you trying to weave in the switch 
+			statement elements here? I don't see the advantage
+			of these macros.
 			
+			FILTER_OUTPUT(1, m_outBuf, m_outputBlockSize, 0);
+			*/
+
 			m_bytePos = m_bitPos = 0;
 		}
 <at>  <at>  -98,5 +105,13  <at>  <at>
(Continue reading)

Permalink | Reply | 0 comments |
headers
Stephen torri | 3 Nov 2003 07:02

[PATCH] cast.cpp - aggregate has a partly bracketed initializer

When doing multi-dimensional arrays always include the 'row' elements in
a set of brackets:

Example of warning:

int base[2][4] {
   1,2,3,4,
   5,6,7,8
};

The compiler is unsure where the end of the first row of 4 items. So to
clear this up you must do:

int base[2][4] {
   { 1,2,3,4 },
   { 5,6,7,8 }
};

Stephen

------------------
PATCH
------------------Index: cast.cpp
===================================================================
RCS file: /cvsroot/cryptopp/c5/cast.cpp,v
retrieving revision 1.1.1.1
diff -U2 -r1.1.1.1 cast.cpp
--- cast.cpp	4 Oct 2002 17:31:42 -0000	1.1.1.1
+++ cast.cpp	3 Nov 2003 05:51:08 -0000
 <at>  <at>  -151,48 +151,63  <at>  <at>
(Continue reading)

Permalink | Reply | 22 comments |
headers
Richard Peters | 4 Nov 2003 18:24
Picon
Picon

Re: [PATCH] cast.cpp - aggregate has a partly bracketed initializer

Throwing an exception here does not make much sense, the
CRYPTO_COMPILE_ASSERT is there to assert the condition _at compile time_,
throwing an exception delays the error until runtime.
The unused code warning is probably generated by the way the compiletime
assert is made, but that doesn't mean that it can be removed.

Richard Peters

----- Original Message ----- 
From: "Stephen torri" <storri <at> torri.org>
To: "cryptopp" <cryptopp-list <at> eskimo.com>
Sent: Monday, November 03, 2003 07:02
Subject: [PATCH] cast.cpp - aggregate has a partly bracketed initializer

The assert statement is reported as being unused. This was test both
with the compiler flags set to '-g' and '-O2'.

Stephen

Index: algparam.h
===================================================================
RCS file: /cvsroot/cryptopp/c5/algparam.h,v
retrieving revision 1.6
diff -U2 -r1.6 algparam.h
--- algparam.h 18 Jul 2003 04:35:30 -0000 1.6
+++ algparam.h 3 Nov 2003 05:21:05 -0000
 <at>  <at>  -24,6 +24,14  <at>  <at> 
  template <class T> ConstByteArrayParameter(const T &string, bool
deepCopy = false)
  {
(Continue reading)

Permalink | Reply | 21 comments |
headers
Stephen torri | 4 Nov 2003 18:52

Re: [PATCH] cast.cpp - aggregate has a partly bracketed initializer

On Tue, 2003-11-04 at 11:24, Richard Peters wrote:
> Throwing an exception here does not make much sense, the
> CRYPTO_COMPILE_ASSERT is there to assert the condition _at compile time_,
> throwing an exception delays the error until runtime.
> The unused code warning is probably generated by the way the compiletime
> assert is made, but that doesn't mean that it can be removed.
> 
> Richard Peters

It seems a bit odd to do this at compile time. That is just my simple
understanding of the code so far. It says that you are accepting in a
const class T type by reference. You then make sure that the lenght of
it is only one character like an array.

It does not seem like you want a single character from the function
signature. It looks like a string is possible with 1+ characters. I
would think it would be better to say:

template <class T> ConstByteArrayParameter (const T &char, bool deepCopy
= false)

With this I know that you are expecting in only a single character.
In the end I don't know where this function would be useful. An example
would be nice to see.

Stephen
--

-- 
Stephen Torri
GPG Key: http://www.cs.wustl.edu/~storri/storri.asc
(Continue reading)

Permalink | Reply | 20 comments |
headers
Wei Dai | 5 Nov 2003 02:15

Re: [PATCH] cast.cpp - aggregate has a partly bracketed initializer

Thanks for the patches. I've fixed the warnings that make sense to fix. 
Unfortunately some of the warnings are not valid, and with GCC there is no 
way to turn off individual warnings, so it won't be possible to get 
Crypto++ to compile without warnings at -Wall.

On Tue, Nov 04, 2003 at 11:52:27AM -0600, Stephen torri wrote:
> It seems a bit odd to do this at compile time. That is just my simple
> understanding of the code so far. It says that you are accepting in a
> const class T type by reference. You then make sure that the lenght of
> it is only one character like an array.

The assert is asserting that the first element of the string has size 1,
in other words, that the string is not a string of wide characters.
Permalink | Reply | 19 comments |
headers
Stephen torri | 5 Nov 2003 05:50

Re: [PATCH] cast.cpp - aggregate has a partly bracketed initializer

On Tue, 2003-11-04 at 19:15, Wei Dai wrote:
> Thanks for the patches. I've fixed the warnings that make sense to fix. 
> Unfortunately some of the warnings are not valid, and with GCC there is no 
> way to turn off individual warnings, so it won't be possible to get 
> Crypto++ to compile without warnings at -Wall.
> 

I was curious to know if this mailing list was the place for them. I
will post more here as I go through the code.

I understand that its not always possible to be warnings free but they
are a good indicator that something might possibly be wrong. So I go
through and kill most if not all of the warnings I see. I find very few
reasons why a warning should be left to show up again.

Stephen
--

-- 
Stephen Torri
GPG Key: http://www.cs.wustl.edu/~storri/storri.asc
Permalink | Reply | 18 comments |
headers
Guy Smith | 5 Nov 2003 15:39

Regarding warnings

Because of the recent discussion about warnings in CryptoPP I decided to
post my thoughts on the subject.  For several months I have felt uneasy
using CryptoPP for several reasons, one of which is the number of warnings
that appear.  Although the warnings have been innocuous (unreferenced formal
parameters, mostly) the strict programmer in me always feels that any
warnings at all are indicative of sloppy programming or (worse) sloppy
design.  They can also indicate incomplete software; I often have code in
intermediate stages of development where I purposely insert code that will
cause warnings so as to remind me that the software is unfinished
(unreferenced formal parameters are useful indicators of function stubs, for
example).  However, these warnings should obviously be eliminated before
code is released.

Because CryptoPP is used for sensitive purposes I am increasingly convinced
that the strict, conservative interpretation of warnings is vital.
Cryptography is one of the few things we know how to do right (who am I
quoting here?), so it doesn't make sense to accept software that isn't
perfect.  Additionally, from a confidence standpoint it doesn't help to have
such crucial code spewing warnings to the developer; I am relying on this
code to be secure.  I cannot use code that is sloppy or unfinished.

This brings me to my second concern about the library, which is its'
extremely large size and opaqueness.  While I normally appreciate
comprehensive libraries, in the case of cryptography I have different
concerns.  My concern is to have a handful of primitive functions that I can
access directly, which I can audit for reliability, and which are cleanly
modularized.  These requirements mean that what I want is a library that is
NOT implemented using generic programming techniques.  I am a big fan of
generic programming in other endeavors (I use it every day) but one of its'
big drawbacks is that it makes code harder to read.  I simply don't have
(Continue reading)

Permalink | Reply | 16 comments |

Gmane