[asterisk-dev] strange libss7 coding style!

[tian]

Thank you Donny, I will try one of them.


From: Donny Kavanagh 
Sent: Friday, July 24, 2009 11:31 AM
To: Asterisk Developers Mailing List 
Subject: Re: [asterisk-dev] strange libss7 coding style!


You could run one of the numerous free Virtual Machine packages, Vmware Player, Sun Virtual Box, and run a Linux machine contained within your windows machine :)

Donny


On Thu, Jul 23, 2009 at 11:07 PM, tian <tian00di00 at live.com> wrote:

  I am NOT a programmer, I am even running Windows instead of Linux. I have
  some knowledge of C and I am reading through libss7 source code to learn
  about the internals of SS7 signaling. I don't know how to submit a patch but
  I have found a couple of errors in the libss7 source code, how can a person
  like me (some sort of a code reviewer but not a technician) to send
  feedbacks? Is there any guideline? Thanks!

  p.s. I believe the following libss7 code contain bugs:

  Version SVN
  SVN Trunk
  SVN Revision 269

  Source file: mtp3.c

  Lines: 470-476

  if (LINKSET_UP_DELAY > -1) { // #define LINKSET_UP_DELAY 500
     if (ss7->linkset_up_timer > -1)
         ss7_schedule_del(ss7, &ss7->linkset_up_timer);
     ss7->linkset_up_timer = ss7_schedule_event(ss7, LINKSET_UP_DELAY,
  &linkset_up_expired, ss7);
     ss7_message(ss7, "LINKSET UP DELAYING RESETTING\n");
  } else // **** this clause will NEVER be excecuted!
     ss7_linkset_up_event(ss7);


  Description of the bug: since LINKSET_UP_DELAY is #define'd to be 500, the
  if condition is always true so the else clause will never be executed!


  Version SVN
  SVN Trunk
  SVN Revision 269

  Source file: mtp3.c

  Lines: 452-464

  /* try force uninhibit */
  if (i == ss7->numlinks) {
     for (i = 0; i < ss7->numlinks; i++) {
         if (ss7->links[i]->inhibit & INHIBITED_REMOTELY) {
             if (!(ss7->links[i]->got_sent_netmsg & SENT_LFU))
                 break;  // **** This break should be deleted! ****
             AUTORL(rl, ss7->links[i]);
             net_mng_send(ss7->links[i], NET_MNG_LFU, rl, 0);
             ss7_message(ss7, "Forced uninhibiting remotely inhibited link
  (no more signalling links are in service) SLC: %i ADJPC: %i\n",
  ss7->links[i]->slc, ss7->links[i]->dpc);
             break;
         }
     }
  }

  Bug description: the first 'break' keyword is misused and should be deleted!
  Apparently the logic of the program is to check whether a LFU has been sent
  to the far end, and if it is not the case, send a LFU, but the 'break'
  breaks the logic.


  I have some other bugs, if positive replies are received, I am glad to
  report them (In the hope that installing a Linux box is not required in the
  process of bug reporting).

  Regard,

  Tian


  --------------------------------------------------
  From: "Russell Bryant" <russell at digium.com>
  Sent: Friday, July 24, 2009 8:32 AM
  To: "Asterisk Developers Mailing List" <asterisk-dev at lists.digium.com>
  Subject: Re: [asterisk-dev] strange libss7 coding style!


  > tian wrote:
  >> Hi, guys!
  >
  > Hi!
  >
  > Please create a new message instead of reply to another one when
  > starting a new thread.
  >
  >> suppose that A, B, C and D are some boolean expressions, the libss7 code
  >> used something like this:
  >>
  >> if (  (A && B) ? 1 : (C || D)  )
  >>     // statements when condition is true
  >>
  >>
  >> I think the following would be equivalent and more clean
  >>
  >> if (   (A && B) || (C || D)   )
  >>     // statements when condition is true
  >>
  >>
  >> I think this is strange coding style and am wondering what benefit can we
  >> get from this? Do you?
  >
  > Agreed.  Patches that simplify logic like this are welcome.
  >
  > --
  > Russell Bryant
  > Digium, Inc. | Engineering Manager, Open Source Software
  > 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
  > Check us out at: www.digium.com & www.asterisk.org
  >
  > _______________________________________________
  > --Bandwidth and Colocation Provided by http://www.api-digital.com--
  >
  > asterisk-dev mailing list
  > To UNSUBSCRIBE or update options visit:
  >   http://lists.digium.com/mailman/listinfo/asterisk-dev
  >

  _______________________________________________
  --Bandwidth and Colocation Provided by http://www.api-digital.com--

  asterisk-dev mailing list
  To UNSUBSCRIBE or update options visit:
    http://lists.digium.com/mailman/listinfo/asterisk-dev





--------------------------------------------------------------------------------


_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.com--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.digium.com/pipermail/asterisk-dev/attachments/20090724/06f13531/attachment-0001.htm