Code cleanup

[NSS Ltd]

I've run some static analysis on the aox codebase and it's thrown a few
potential issues up but looking at the code, it looks like some of the
things highlighted are intentional.

So, rather than jumping in, I wanted to check the background on some of
them :


Missing break (especially as previous cases do have breaks) ?

server/selector.cpp|1976|redundantAssignInSwitch : style : Variable 'w'
is reassigned a value before the old one has been used. 'break;' missing?|

    case Modseq:
        w = "modseq";
        break;
    case Age:
        w = "age";
    case DatabaseId:
        w = "database-id";
    case ThreadId:
        w = "thread-id";
    };


I wonder if the byte order is wrong here :

encodings/utf.cpp|238|duplicateBranch : style : Finding the same code in
an 'if' and related 'else' branch is suspicious and might indicate a cut
and paste or logic error. Please examine this code carefully to
determine if it is correct.|


EString Utf16Codec::fromUnicode( const UString & u )
{
    EString r;

    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( 0xff );
    }
    else {
        r.append( 0xfe );   // Should this be  0xff then 0xfe instead ? 
different byte order ?
        r.append( 0xff );    // or should the other branch be swapped ?
    }

    if ( be )
        r.append( (new Utf16BeCodec)->fromUnicode( u ) );
    else
        r.append( (new Utf16LeCodec)->fromUnicode( u ) );

    return r;
}


This looks like it's a minor fix :

core/allocator.cpp|919|invalidPrintfArgType_sint : style : %d in format
string (no. 4) requires a signed integer given in the argument list.|

                            uint n = 0;
                            while ( n < number ) {
                                if ( m->payload[n] == p ) {
                                    fprintf( stderr,
                                             "Pointer at 0x%p (in 0x%p, "
                                             "size <= %d, %d pointers)\n",
                                             &m->payload[n],
                                             &m->payload[0],
                                             a->step - bytes,
                                             number );


This one looks like a redundant check, given that d is dereferenced
previously in the code block :

core/estring.cpp|1816|nullPointer : style : Possible null pointer
dereference: d - otherwise it is redundant to check it against null.|


There's quite a few of these (this is one example) - I'm wondering if
this is to deal with a compiler quirk (but I expect it would be
optimized out at compile time) :

installer/installer.cpp|2293|unreadVariable : style : Variable 'ignore'
is assigned a value that is never used.|

            ignore = ignore;

There are also variations like this too :

server/session.cpp|314|selfAssignment : style : Redundant assignment of
't' to itself.|

void Session::emitUpdates( Transaction * t )
{
    t = t;
}

I don't see any = operator definition on Transaction class and this is a
pointer, so this does not seem to have any special side effect defined
so looks redundant unless it's doing some magic?


This looks like it's what it says :

message/mimefields.cpp|380|duplicateIf : style : Duplicate conditions in
'if' and related 'else if'. This is suspicious and might indicate a cut
and paste or logic error. Please examine this code carefully to
determine if it is correct.|

            // the remainder is from RFC 1049
            else if ( s == "postscript" ) {
                t = "application";
                st = "postscript";
            }
            else if ( s == "postscript" ) {
                t = "application";
                st = "postscript";
            }
            else if ( s == "sgml" ) {



There are some exception and pointer assignment warnings but they look
quite minor.

Before doing any fixing on these, I wanted to get some input from Arnt
and others more familiar with any reasons for things being like this.

Thanks
Jim