Loading ...
Sorry, an error occurred while loading the content.
 

Yet another bug (in the orders checker, this time)

Expand Messages
  • House of Tzarg (176)
    Atlantis Guru s, There is a potentially fatal bug in the OrdersCheck object, because it doesn t clean up properly, and OrdersCheck::dummyUnit to delete its
    Message 1 of 6 , Nov 2 6:55 AM
      Atlantis Guru's,

      There is a potentially fatal bug in the OrdersCheck object, because it
      doesn't clean up properly, and OrdersCheck::dummyUnit to delete its
      monthorders pointer when the target has already been released by the
      OrdersCheck destructor.

      When the OrdersCheck object is constructed, it also constructs Unit
      dummyUnit and Order dummyOrder. When an order is found, it is placed
      in dummyOrder, which is then referenced by dummyUnit.monthorders. When
      the OrdersCheck object is freed (on exit from Game::DoOrdersCheck),
      dummyOrder is freed, then dummyUnit, which tries to delete its
      monthorders pointer - which usually points to dummyOrder, which has
      already been deallocated - causing a fatal error.

      The fix is simple:
      In parseorders.cpp, add the following destructor:
      OrdersCheck::~OrdersCheck()
      {
      dummyUnit.ClearOrders(); // dummyUnit.monthorders=0 also works
      }
      And declare it in game.h as follows:
      class OrdersCheck
      {
      public:
      OrdersCheck();
      virtual ~OrdersCheck();

      Aoutfile *pCheckFile;
      Unit dummyUnit;
      Faction dummyFaction;
      Order dummyOrder;
      int numshows;

      void Error( const AString &error );
      };

      For safety's sake, in parseorders.cpp, Game::ParseOrders, edit:
      <snip>
      case O_END:
      pCheck->dummyUnit.ClearOrders(); // add this line
      <snip>
      case O_UNIT:
      pCheck->dummyUnit.ClearOrders(); // add this line
      <snip>
    • JT
      ... I posted this patch to this newsgroup quite a while back (or a similar patch with the same effect :) You might want to look back through archives so you
      Message 2 of 6 , Nov 2 8:59 AM
        On Thu, 2 Nov 2000, House of Tzarg (176) wrote:
        > There is a potentially fatal bug in the OrdersCheck object, because it
        > doesn't clean up properly, and OrdersCheck::dummyUnit to delete its
        > monthorders pointer when the target has already been released by the
        > OrdersCheck destructor.

        I posted this patch to this newsgroup quite a while back (or a similar
        patch with the same effect :) You might want to look back through
        archives so you don't patch and refix bugs which are already known about
        :) [As a small side note, this bug got found because it caused the
        checker to crash on my linux box whereas it worked fine on an NT/windows
        box]

        --JT


        --
        [-------------------------------------------------------------------------]
        [ Practice random kindness and senseless acts of beauty. ]
        [ It's hard to seize the day when you must first grapple with the morning ]
        [-------------------------------------------------------------------------]
      • ido bach
        are all these fixes inserted into the linux version on the archives ?.. i mean , never encountered any because i use windows .. but i hate to think i ll need
        Message 3 of 6 , Nov 2 12:17 PM
          are all these fixes inserted into the linux version
          on the archives ?.. i mean , never encountered any
          because i use windows .. but i hate to think i'll
          need to gather all these fixes around if i change
          systems ..

          so , if you can write the fixes on txt files
          and upload them to the files section
          might be a great help in the future .. who knows .
        • House of Tzarg (176)
          ... similar ... Look back over 500 messages? Not bloody likely! Actually, I did, to see what bug fixes were necessary, but I must have missed your posting.
          Message 4 of 6 , Nov 3 5:31 AM
            > I posted this patch to this newsgroup quite a while back (or a
            similar
            > patch with the same effect :) You might want to look back through
            > archives so you don't patch and refix bugs which are already known
            > about :)

            Look back over 500 messages? Not bloody likely!
            Actually, I did, to see what bug fixes were necessary, but I must have
            missed your posting. Sorry :(

            > [As a small side note, this bug got found because it caused the
            > checker to crash on my linux box whereas it worked fine on an
            > NT/windows box]

            That's how I found it too (Linux). Just because Windoze doesn't report
            these errors in release code doesn't mean they aren't still there. Run
            it on Windoze in the degbugger, and you find the bug immediately. And
            probably loads more besides.

            And another thing: I messed up the fix. The lines:
            pCheck->dummyUnit.ClearOrders(); should be:
            if (pCheck) pCheck->dummyUnit.ClearOrders();
            as below:
            > In parseorders.cpp, add the following destructor:
            > OrdersCheck::~OrdersCheck()
            > {
            > dummyUnit.ClearOrders(); // dummyUnit.monthorders=0 also works
            > }
            > And declare it in game.h as follows:
            > class OrdersCheck
            > {
            > public:
            > OrdersCheck();
            > virtual ~OrdersCheck();
            >
            > Aoutfile *pCheckFile;
            > Unit dummyUnit;
            > Faction dummyFaction;
            > Order dummyOrder;
            > int numshows;
            >
            > void Error( const AString &error );
            > };
            >
            > For safety's sake, in parseorders.cpp, Game::ParseOrders, edit:
            > <snip>
            > case O_END:
            > if (pCheck) pCheck->dummyUnit.ClearOrders(); // add this line
            > <snip>
            > case O_UNIT:
            > if (pCheck) pCheck->dummyUnit.ClearOrders(); // add this line
          • House of Tzarg (176)
            ... Hmmm... You need them on Windoze too. You just don t realise it. The code is portable, and should be the same for all operating systems, with the exception
            Message 5 of 6 , Nov 3 5:35 AM
              --- In atlantisdev@egroups.com, "ido bach" <nili1@b...> wrote:
              >
              > are all these fixes inserted into the linux version
              > on the archives ?.. i mean , never encountered any
              > because i use windows .. but i hate to think i'll
              > need to gather all these fixes around if i change
              > systems ..

              Hmmm... You need them on Windoze too. You just don't realise it.

              The code is portable, and should be the same for all operating
              systems, with the exception of make/project files which will be
              compiler/linker specific.

              > so , if you can write the fixes on txt files
              > and upload them to the files section
              > might be a great help in the future .. who knows .

              Good idea. I'll make a start. And the rest of you out there can fix
              all my mistakes and omissions.

              Cheers,
              Tom
              (aka Tzarg)
            • JT
              ... It s okay.. I just was pointing out that doing so might be useful as it might prevent duplication (searching the list for bug and patch should drop
              Message 6 of 6 , Nov 3 9:10 AM
                On Fri, 3 Nov 2000, House of Tzarg (176) wrote:
                > Look back over 500 messages? Not bloody likely! Actually, I did, to
                > see what bug fixes were necessary, but I must have missed your
                > posting. Sorry :(

                It's okay.. I just was pointing out that doing so might be useful as it
                might prevent duplication (searching the list for 'bug' and 'patch' should
                drop that 500 down to a manageble number :)

                > > [As a small side note, this bug got found because it caused the
                > > checker to crash on my linux box whereas it worked fine on an
                > > NT/windows box]
                >
                > That's how I found it too (Linux). Just because Windoze doesn't report
                > these errors in release code doesn't mean they aren't still there. Run
                > it on Windoze in the degbugger, and you find the bug immediately. And
                > probably loads more besides.

                I know.. I don't actually run the code on NT, but I know someone who does
                who was insisting this was a linux only problem :/ (which is why I
                mentioned that the bug still existed even though it worked fine)

                --JT

                --
                [-------------------------------------------------------------------------]
                [ Practice random kindness and senseless acts of beauty. ]
                [ It's hard to seize the day when you must first grapple with the morning ]
                [-------------------------------------------------------------------------]
              Your message has been successfully submitted and would be delivered to recipients shortly.