Here's a patch with the cleanups I think make sense. These may
duplicate some Abhijit already has in the pipeline from my previous email.
I've not looked at the pointer conversion or memory allocation things
which were flagged. I don't think these are a problem that needs
attention (at least not now). After a little research, many are flagged
because the C++ spec has changed with
nullptr/static_cast/dynamic_cast/const_cast/reinterpret_cast and the old
C style is discouraged as a result.
http://www.cplusplus.com/doc/tutorial/typecasting/
Jim
On 22/06/2016 17:30, NSS Ltd wrote:
> I ran cppcheck. It reports a number of other things which are more
> minor and to do with code style and compliance to C++ specifications.
> (e.g. C style pointer conversions, re-throwing exceptions etc.), unused
> methods and failure to initialize members in constructors.
>
> I've not gone through it exhaustively; initially I wanted to check after
> some of my patches (the guid stuff) and I just scanned for what looked
> most out of place and worth investigating.
>
diff --git a/aox/db.cpp b/aox/db.cpp
index 24a7dca..af5edbb 100644
--- a/aox/db.cpp
+++ b/aox/db.cpp
@@ -173,6 +173,9 @@ void Vacuum::execute()
qstate = 1;
log( "vacuum: delete from deliveries", Log::Significant );
do {
+ // Literals have changed meaning in c++11, we need spaces
+ // or they have potentially changed effect in future
+ // http://www.preney.ca/paul/archives/636
q = new Query( "delete from deliveries "
"where injected_at<current_timestamp-'" +
fn( days ) + " days'::interval "
diff --git a/core/allocator.cpp b/core/allocator.cpp
index 8571074..42c6a50 100644
--- a/core/allocator.cpp
+++ b/core/allocator.cpp
@@ -432,7 +432,7 @@ void Allocator::deallocate( void * p )
random binary data.
Scanning long strings is slow. Binary data can give false alarms
- during pointer scanning, which will lead ot memory not being
+ during pointer scanning, which will lead to memory not being
freed.
*/
@@ -918,7 +918,7 @@ void pointers( void * p )
if ( m->payload[n] == p ) {
fprintf( stderr,
"Pointer at 0x%p (in 0x%p, "
- "size <= %d, %d pointers)\n",
+ "size <= %d, %u pointers)\n",
&m->payload[n],
&m->payload[0],
a->step - bytes,
diff --git a/core/file.cpp b/core/file.cpp
index 70451d6..9f8f7db 100644
--- a/core/file.cpp
+++ b/core/file.cpp
@@ -22,8 +22,6 @@
#endif
-extern "C" void *memcpy(void *, const void *, uint);
-
class FileData
: public Garbage
diff --git a/db/query.cpp b/db/query.cpp
index 966e495..70d1bbe 100644
--- a/db/query.cpp
+++ b/db/query.cpp
@@ -562,7 +562,7 @@ void Query::notify()
try {
d->owner->execute();
}
- catch ( Exception e ) {
+ catch ( const Exception& e ) {
d->owner = 0; // so we can't get close to a segfault again
if ( e == Invariant ) {
setError( "Invariant failed while processing Query::notify()" );
@@ -599,7 +599,7 @@ void Query::notify()
}
}
else {
- throw e;
+ throw; // Re-throw without creating a copy (throw e makes a
redundant copy)
}
}
}
diff --git a/db/transaction.cpp b/db/transaction.cpp
index a85a7b5..d33ee4e 100644
--- a/db/transaction.cpp
+++ b/db/transaction.cpp
@@ -609,7 +609,7 @@ void Transaction::notify()
try {
d->owner->execute();
}
- catch ( Exception e ) {
+ catch ( const Exception& e ) {
d->owner = 0; // so we can't get close to a segfault again
if ( e == Invariant ) {
setError( 0,
@@ -633,7 +633,7 @@ void Transaction::notify()
}
}
else {
- throw e;
+ throw; // Rethrow without creating redundant copy (which throw e
does)
}
}
if ( done() && d->parent &&
diff --git a/encodings/utf.cpp b/encodings/utf.cpp
index 66bf4f2..7fc5cd1 100644
--- a/encodings/utf.cpp
+++ b/encodings/utf.cpp
@@ -230,18 +230,20 @@ EString Utf16Codec::fromUnicode( const UString & u )
{
EString r;
+ //
http://stackoverflow.com/questions/701624/difference-between-big-endian-and-little-endian-byte-order
+
if ( !bom ) {
// if we don't output a BOM, reader should assume BE, so we
// must be BE to conform
be = true;
}
else if ( be ) {
- r.append( 0xfe );
+ r.append( 0xfe ); // Big Endian UTF16-BE
r.append( 0xff );
}
else {
+ r.append( 0xff ); // Little Endian UTF16-LE
r.append( 0xfe );
- r.append( 0xff );
}
if ( be )
diff --git a/message/mimefields.cpp b/message/mimefields.cpp
index ccf3009..afd664a 100644
--- a/message/mimefields.cpp
+++ b/message/mimefields.cpp
@@ -381,10 +381,6 @@ void ContentType::parse( const EString &s )
t = "application";
st = "postscript";
}
- else if ( s == "postscript" ) {
- t = "application";
- st = "postscript";
- }
else if ( s == "sgml" ) {
t = "text";
st = "sgml";
diff --git a/server/eventloop.cpp b/server/eventloop.cpp
index 0db13e3..6e0953f 100644
--- a/server/eventloop.cpp
+++ b/server/eventloop.cpp
@@ -369,7 +369,7 @@ void EventLoop::start()
c->log( "Still have " +
EString::humanNumber( c->writeBuffer()->size() ) +
" bytes to write", Log::Debug );
- } catch ( Exception e ) {
+ } catch ( const Exception& e ) {
// we don't really care at this point, do we?
}
}
@@ -501,7 +501,7 @@ void EventLoop::dispatch( Connection * c, bool r, bool w,
uint now )
s && s == c->writeBuffer()->size() )
c->writeBuffer()->remove( s );
}
- catch ( Exception e ) {
+ catch ( const Exception& e ) {
EString s;
switch (e) {
case Invariant:
@@ -557,7 +557,7 @@ void EventLoop::stop( uint s )
else if ( s <= 1 && !c->hasProperty( Connection::Internal ) ) {
c->react( Connection::Shutdown );
}
- } catch ( Exception e ) {
+ } catch ( const Exception& e ) {
removeConnection( c );
}
}
diff --git a/server/selector.cpp b/server/selector.cpp
index 1b68a2a..c104bf8 100644
--- a/server/selector.cpp
+++ b/server/selector.cpp
@@ -1972,10 +1972,13 @@ EString Selector::debugString() const
break;
case Age:
w = "age";
+ break;
case DatabaseId:
w = "database-id";
+ break;
case ThreadId:
w = "thread-id";
+ break;
};
r = w + " " + o + " ";
diff --git a/server/server.cpp b/server/server.cpp
index b853071..4cd69ee 100644
--- a/server/server.cpp
+++ b/server/server.cpp
@@ -207,7 +207,7 @@ void Server::setup( Stage s )
}
d->stage = (Stage)(d->stage + 1);
}
- } catch ( Exception e ) {
+ } catch ( const Exception& e ) {
// don't allocate memory or call anything here.
const char * c = 0;
switch (e) {
diff --git a/server/timer.cpp b/server/timer.cpp
index f30a381..37f432b 100644
--- a/server/timer.cpp
+++ b/server/timer.cpp
@@ -135,7 +135,7 @@ void Timer::notify()
try {
d->owner->execute();
}
- catch ( Exception e ) {
+ catch ( const Exception& e ) {
d->owner = 0; // so we can't get close to a segfault again
if ( e == Invariant ) {
// Analogous to EventLoop::dispatch, we try to close the
@@ -161,7 +161,7 @@ void Timer::notify()
}
}
else {
- throw e;
+ throw; // Re-throws caught exception
}
}
}
