Re[2]: [lmi] questionable code checking for e_currbasis in ihs_acctval.cpp

[Vadim Zeitlin]

On Mon, 24 Mar 2008 17:10:19 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2008-03-24 04:31Z, Vadim Zeitlin wrote:
GC> > 
GC> >  There are 4 occurrences of the following expression in ihs_acctval.cpp:
GC> > 
GC> >         !e_currbasis == ExpAndGABasis
GC> > 
GC> > which attracted my attention because MSVC complained about 
GC> > 
GC> >   '==' : unsafe mix of type 'bool' and type 'int' in operation
GC> 
GC> That demonstrates the value of using another compiler.

 ... And of using maximal warning level for all the compilers used.
Unfortunately I had to disable several VC warnings for now as I get ~3000
of them in LMI code. But I do plan to work on enabling them back and fixing
them once the main VC compilation patch is applied. BTW, I assume that
you want me to submit it via Savannah patch tracker too, please let me know
if you prefer some other form of submission for it (if you don't reply,
I'll put it there, hopefully tonight).

 BTW, here is a patch to fix another (albeit less serious one as the
current code works correctly, if only by accident) bug uncovered by VC7
warning:

diff -r 5f2af06861b4 ihs_irc7702.cpp
--- a/ihs_irc7702.cpp   Mon Mar 24 04:18:41 2008 +0100
+++ b/ihs_irc7702.cpp   Mon Mar 24 19:06:55 2008 +0100
@@ -803,19 +803,13 @@ Irc7702::EIOBasis Irc7702::Get4PctBasis
 {
     switch(a_DBOpt)
         {
-        case e_option1:
+        case e_option1_for_7702:
             {
             return Opt1Int4Pct;
             }
-        case e_option2:
+        case e_option2_for_7702:
             {
             return Opt2Int4Pct;
-            }
-        case e_rop:
-            {
-            // TODO ?? Wait...shouldn't this be the 7702 equivalent
-            // for ROP?
-            return Opt1Int4Pct;
             }
         default:
             {

 Here the problem is that a_DBOpt is of type e_dbopt_7702 but the values in
the switch are of different enum_dbopt type. While the 2 enums do have the
same values for their first 2 elements, there is no e_rop for e_dbopt_7702
case and so the TODO can be trivially fixed while the constants in the case
should be changed to compare the values of the same types only.

 Regards,
VZ