#713 AMF refactoring for 4.5

4.5.0
fixed
nobody
None
enhancement
amf
-
4.4
major
2014-09-30
2014-01-10
Hans Feldt
No

This is a 4.5 ticket for continued code re-factoring of the AMF service for 4.5. The work started with #94 in 4.4.

So far no really no C++ features has been used. Files has been renamed so that the C++ compiler is used and new/delete is used instead of malloc/free (where appropriate).

In this ticket it is time to make use of C++ features to get a more maintainable and understandable code base. We should aim for many small changes instead of few big ones.

An ordered list of things to work on:
1. Use bool which is a native type in C++ (and remove SaBoolT)
2. Reduce number of casts in the code (introduced because of C++ files)
2. Use stl::maps (instead of patricia trees)
3. Use stl::vector/list (instead of legacy/home made lists)

Other yet non prioritized items:
- Remove use of EDU and do direct encode/decode
- Clean run of google's cpplint on the code base
- Convert model derived C structs to classes and change functions into methods
- Change macros to (inline) methods
- Change bit fields (flags) to boolean attributes (see https://sourceforge.net/p/opensaf/tickets/717/#d4b2)
- Use references instead of pointers
- ...

Non C++ related changes:
- split up long functions into smaller ones doing one things and not many
- use pmccabe complexity analysis to aid refactoring
- set a goal for pmccabe complexity
- use const as much as possible
- ...

Related

Tickets: #713

Discussion

1 2 3 > >> (Page 1 of 3)
  • Hans Feldt

    Hans Feldt - 2014-01-13

    changeset: 4810:7938a6a570f4
    parent: 4806:d8a9d45dfbdf
    user: Hans Feldt osafdevel@gmail.com
    date: Fri Jan 10 15:43:33 2014 +0100
    summary: amfnd: remove unneeded lock [#713]

    changeset: 4811:eb57695a171b
    tag: tip
    user: Hans Feldt osafdevel@gmail.com
    date: Fri Jan 10 15:43:34 2014 +0100
    summary: amf: remove amf_ipc.h [#713]

     

    Related

    Tickets: #713

  • Hans Feldt

    Hans Feldt - 2014-01-27
    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -4,11 +4,13 @@
    
     In this ticket it is time to make use of C++ features to get a more maintainable and understandable code base. We should aim for many small changes instead of few big ones.
    
    -Some things to consider for the C++ conversion:
    +An ordered list of things to work on:
    +1. Use bool which is a native type in C++ (and remove SaBoolT)
    +2. Use stl::maps (instead of patricia trees)
    +3. Use stl::vector/list (instead of legacy/home made lists)
    +
    +Other yet non prioritized items:
     - Clean run of google's cpplint on the code base
    -- Use STL containers
    -    - patricia tree converted to maps
    -    - linked links converted to vectors (or lists when needed)
     - Convert model derived C structs to classes and change functions into methods
     - Change macros to (inline) methods
     - Change bit fields (flags) to boolean attributes
    @@ -17,7 +19,8 @@
    
     Non C++ related changes:
     - split up long functions into smaller ones doing one things and not many
    +- use pmccabe complexity analysis to aid refactoring
    +- set a goal for pmccabe complexity
     - use const as much as possible
     - ...
    
    -This is just a starting point, please comment
    
     
  • Hans Feldt

    Hans Feldt - 2014-01-31
    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -13,7 +13,7 @@
     - Clean run of google's cpplint on the code base
     - Convert model derived C structs to classes and change functions into methods
     - Change macros to (inline) methods
    -- Change bit fields (flags) to boolean attributes
    +- Change bit fields (flags) to boolean attributes (see https://sourceforge.net/p/opensaf/tickets/717/#d4b2)
     - Use references instead of pointers
     - ...
    
     
  • Hans Feldt

    Hans Feldt - 2014-02-04
    • Type: defect --> enhancement
     
  • Anders Widell

    Anders Widell - 2014-02-05

    One big improvement in C++ is the possibility to use std::string instead of C character arrays. C++ strings are much less error-prone, e.g. you can almost eliminate the risk for "buffer overflow" type of errors in string handling (which is still a common cause of security vulnerabilities in many applications), and if you use the error-checking at() method for accessing individual characters, you will get an out_of_range exception if you try to access a position that is out of bounds. Furthermore, it manages dynamic memory allocation internally, which eliminates the need for malloc() & free() and thus the risk for memory leaks. In C, it is all too common to use a fixed-size buffer for storing strings, just to avoid the malloc() / free() hazzle. Using a fixed-size buffer is not a good alternative, since it introduces an arbritrary limit on the maximum string length, and it wastes memory since most of the time only a small fraction of the buffer will be utilized.

    Since we are planning to increase (or remove) the length limit for DNs, we need to eliminate the usage of SaNameT internally in the OpenSAF services. Here, std::string also comes in handy as a replacement for SaNameT. SaNameT should only be used where it is absolutely necessary, i.e. in APIs due to backward compability reasons.

     
    • Anders Bjornerstedt

      Agree with this.

      You mentioned c++ exceptions.
      Lets not start writing code for catching exceptions for now, since that can have quite heavy performance/stack-memory implications.
      That means thrown exceptions will be like asserts.

      /AndersBj


      From: Anders Widell [mailto:anders-w@users.sf.net]
      Sent: den 5 februari 2014 12:28
      To: opensaf-tickets@lists.sourceforge.net
      Subject: [tickets] [opensaf:tickets] #713 AMF refactoring for 4.5

      One big improvement in C++ is the possibility to use std::string instead of C character arrays. C++ strings are much less error-prone, e.g. you can almost eliminate the risk for "buffer overflow" type of errors in string handling (which is still a common cause of security vulnerabilities in many applications), and if you use the error-checking at() method for accessing individual characters, you will get an out_of_range exception if you try to access a position that is out of bounds. Furthermore, it manages dynamic memory allocation internally, which eliminates the need for malloc() & free() and thus the risk for memory leaks. In C, it is all too common to use a fixed-size buffer for storing strings, just to avoid the malloc() / free() hazzle. Using a fixed-size buffer is not a good alternative, since it introduces an arbritrary limit on the maximum string length, and it wastes memory since most of the time only a small fraction of the buffer will be utilized.

      Since we are planning to increase (or remove) the length limit for DNs, we need to eliminate the usage of SaNameT internally in the OpenSAF services. Here, std::string also comes in handy as a replacement for SaNameT. SaNameT should only be used where it is absolutely necessary, i.e. in APIs due to backward compability reasons.


      [tickets:#713]http://sourceforge.net/p/opensaf/tickets/713/ AMF refactoring for 4.5

      Status: accepted
      Created: Fri Jan 10, 2014 07:05 AM UTC by Hans Feldt
      Last Updated: Tue Feb 04, 2014 06:41 PM UTC
      Owner: Hans Feldt

      This is a 4.5 ticket for continued code re-factoring of the AMF service for 4.5. The work started with #94 in 4.4.

      So far no really no C++ features has been used. Files has been renamed so that the C++ compiler is used and new/delete is used instead of malloc/free (where appropriate).

      In this ticket it is time to make use of C++ features to get a more maintainable and understandable code base. We should aim for many small changes instead of few big ones.

      An ordered list of things to work on:
      1. Use bool which is a native type in C++ (and remove SaBoolT)
      2. Use stl::maps (instead of patricia trees)
      3. Use stl::vector/list (instead of legacy/home made lists)

      Other yet non prioritized items:
      - Clean run of google's cpplint on the code base
      - Convert model derived C structs to classes and change functions into methods
      - Change macros to (inline) methods
      - Change bit fields (flags) to boolean attributes (see https://sourceforge.net/p/opensaf/tickets/717/#d4b2)
      - Use references instead of pointers
      - ...

      Non C++ related changes:
      - split up long functions into smaller ones doing one things and not many
      - use pmccabe complexity analysis to aid refactoring
      - set a goal for pmccabe complexity
      - use const as much as possible
      - ...


      Sent from sourceforge.net because opensaf-tickets@lists.sourceforge.net is subscribed to https://sourceforge.net/p/opensaf/tickets/

      To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/opensaf/admin/tickets/options. Or, if this is a mailing list, you can unsubscribe from the mailing list.

       

      Related

      Tickets: #713
      Tickets: tickets

  • Anders Widell

    Anders Widell - 2014-02-05

    Yes, absolutely. The exceptions thrown by the error checking code in the C++ standard library should terminate the process (which they do unless you catch them). They benefit is that we catch the error immediately when it happens, instead of getting a memory corruption that is much more difficult to debug.

     
  • Hans Feldt

    Hans Feldt - 2014-03-26
    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -6,10 +6,12 @@
    
     An ordered list of things to work on:
     1. Use bool which is a native type in C++ (and remove SaBoolT)
    +2. Reduce number of casts in the code (introduced because of C++ files)
     2. Use stl::maps (instead of patricia trees)
     3. Use stl::vector/list (instead of legacy/home made lists)
    
     Other yet non prioritized items:
    +- Remove use of EDU and do direct encode/decode
     - Clean run of google's cpplint on the code base
     - Convert model derived C structs to classes and change functions into methods
     - Change macros to (inline) methods
    
     
  • Praveen

    Praveen - 2014-04-03

    Please update the ticket with change set number whenever any patch is pushed.

     
  • Hans Feldt

    Hans Feldt - 2014-04-03

    changeset: 5099:484bd069b67a
    user: Hans Feldt hans.feldt@ericsson.com
    date: Thu Apr 03 06:55:48 2014 +0200
    summary: amfd: remove cast from avd_class_impl_set [#713]

    changeset: 5100:8d070ed277b8
    user: Hans Feldt hans.feldt@ericsson.com
    date: Thu Apr 03 06:55:49 2014 +0200
    summary: amfd: remove cast from avd_saImmOiRtObjectUpdate [#713]

    changeset: 5101:0f20bf23274a
    tag: tip
    user: Hans Feldt hans.feldt@ericsson.com
    date: Thu Apr 03 06:55:49 2014 +0200
    summary: amfd: remove cast from avd_saImmOiRtObjectCreate [#713]

     

    Related

    Tickets: #713

  • Hans Feldt

    Hans Feldt - 2014-04-10

    sending [PATCH 0 of 6] Review Request for AMF #713, SU class introduction ...
    sending [PATCH 1 of 6] amfd: change AVD_SU from struct to class [#713] ...
    sending [PATCH 2 of 6] amfd: add and use SU method oper_state_set [#713] ...
    sending [PATCH 3 of 6] amfd: add SU susi manipulation methods [#713] ...
    sending [PATCH 4 of 6] amfd: use SU method delete_all_susis [#713] ...
    sending [PATCH 5 of 6] amfd: use SU methods for changing susis [#713] ...
    sending [PATCH 6 of 6] amfd: add and use SU method readiness_state_set [#713] ...

     

    Related

    Tickets: #713

  • Hans Feldt

    Hans Feldt - 2014-04-11

    changeset: 5148:d3184d131975
    tag: tip
    user: Hans Feldt hans.feldt@ericsson.com
    date: Fri Apr 11 17:30:06 2014 +0200
    summary: amfd: change AVD_SU from struct to class [#713]

     

    Related

    Tickets: #713

  • Hans Feldt

    Hans Feldt - 2014-04-15

    changeset: 5153:6cacbb91f268
    tag: tip
    user: Hans Feldt hans.feldt@ericsson.com
    date: Tue Apr 15 20:56:34 2014 +0200
    summary: amfd: add and use SU method set_readiness_state [#713]

    changeset: 5152:f1110f8ceca9
    user: Hans Feldt hans.feldt@ericsson.com
    date: Tue Apr 15 20:48:52 2014 +0200
    summary: amfd: add and use SU method set_oper_state [#713]

     

    Related

    Tickets: #713

  • Hans Feldt

    Hans Feldt - 2014-04-17

    changeset: 5160:4f45d63e85d5
    user: Hans Feldt hans.feldt@ericsson.com
    date: Thu Apr 17 11:16:22 2014 +0200
    summary: amfd: add SU susi manipulation methods [#713]

    changeset: 5161:71433543f9d8
    user: Hans Feldt hans.feldt@ericsson.com
    date: Thu Apr 17 11:17:53 2014 +0200
    summary: amfd: use SU method delete_all_susis [#713]

    changeset: 5162:30fd01ccdca9
    user: Hans Feldt hans.feldt@ericsson.com
    date: Thu Apr 17 11:18:14 2014 +0200
    summary: amfd: use SU methods for changing susis [#713]

    changeset: 5163:a321898ef7b6
    tag: tip
    user: Hans Feldt hans.feldt@ericsson.com
    date: Thu Apr 17 11:22:22 2014 +0200
    summary: amfd: fix trailing whitespace in makefile [#713]

     

    Related

    Tickets: #713

  • Hans Feldt

    Hans Feldt - 2014-04-22

    changeset: 5166:b62e0608c2db
    tag: tip
    user: Hans Feldt hans.feldt@ericsson.com
    date: Tue Apr 22 07:06:52 2014 +0200
    summary: amfd: use db_template for the SU db [#713]

     

    Related

    Tickets: #713

  • Hans Feldt

    Hans Feldt - 2014-04-29

    changeset: 5197:3d161b89220f
    user: Hans Feldt hans.feldt@ericsson.com
    date: Tue Apr 29 17:04:52 2014 +0200
    summary: amfd: cleanup SU [#713]

    changeset: 5198:11f228e1609a
    user: Hans Feldt hans.feldt@ericsson.com
    date: Tue Apr 29 17:05:42 2014 +0200
    summary: amfd: add and use SU method set_pres_state [#713]

    changeset: 5199:7d15f889f879
    user: Hans Feldt hans.feldt@ericsson.com
    date: Tue Apr 29 17:05:43 2014 +0200
    summary: amfd: add and use SU method set_admin_state [#713]

    changeset: 5200:6b3a429e78ac
    user: Hans Feldt hans.feldt@ericsson.com
    date: Tue Apr 29 17:05:44 2014 +0200
    summary: amfd: add and use SU methods add_comp/remove_comp [#713]

    changeset: 5201:aba1ddcfdb69
    tag: tip
    user: Hans Feldt hans.feldt@ericsson.com
    date: Tue Apr 29 22:54:29 2014 +0200
    summary: amfd: add and use SU method hastate_assignments_count [#713]

     

    Related

    Tickets: #713

  • Hans Feldt

    Hans Feldt - 2014-04-30
    • status: accepted --> review
     
  • Hans Feldt

    Hans Feldt - 2014-04-30

    sending [PATCH 0 of 7] Review Request for AMF SU refactoring #713 ...
    sending [PATCH 1 of 7] amfd: add and use SU assignment count methods [#713] ...
    sending [PATCH 2 of 7] amfd: add and use set_su_failover [#713] ...
    sending [PATCH 3 of 7] amfd: add and use SU method set_term_state [#713] ...
    sending [PATCH 4 of 7] amfd: add SU constr/destr [#713] ...
    sending [PATCH 5 of 7] amfd: add and use set_su_switch [#713] ...
    sending [PATCH 6 of 7] amfd: add and use SU get_node_ptr [#713] ...
    sending [PATCH 7 of 7] amfd: add and use SU is_in_service [#713] ...

     

    Related

    Tickets: #713

  • Hans Feldt

    Hans Feldt - 2014-05-02

    changeset: 5204:e7300228e1fc
    tag: tip
    user: Hans Feldt hans.feldt@ericsson.com
    date: Fri May 02 10:43:36 2014 +0200
    summary: amfd: add and use SU method set_term_state [#713]

    changeset: 5203:ac7703f19fd2
    user: Hans Feldt hans.feldt@ericsson.com
    date: Fri May 02 10:41:14 2014 +0200
    summary: amfd: add and use set_su_failover [#713]

    changeset: 5202:81c80fe7b343
    user: Hans Feldt hans.feldt@ericsson.com
    date: Fri May 02 10:34:04 2014 +0200
    summary: amfd: add and use SU assignment count methods [#713]

     

    Related

    Tickets: #713

  • Hans Feldt

    Hans Feldt - 2014-05-13

    changeset: 5242:e6fc25d095b9
    tag: tip
    user: Hans Feldt hans.feldt@ericsson.com
    date: Tue May 13 07:46:32 2014 +0200
    summary: amfd: fix SU constr [#713]

     

    Related

    Tickets: #713

  • Hans Feldt

    Hans Feldt - 2014-05-16

    changeset: 5271:89fbcd62d959
    parent: 5269:afe69cc9deef
    user: Hans Feldt hans.feldt@ericsson.com
    date: Fri May 16 14:40:23 2014 +0200
    summary: amfd: create sutcomptype header file [#713]

    changeset: 5272:c57407b63382
    user: Hans Feldt hans.feldt@ericsson.com
    date: Fri May 16 14:40:24 2014 +0200
    summary: amfd: add comp_is_preinstantiable [#713]

    changeset: 5273:dc0f82904287
    user: Hans Feldt hans.feldt@ericsson.com
    date: Fri May 16 14:40:24 2014 +0200
    summary: amfd: move supreinst logic from comp to su [#713]

    changeset: 5274:817be0ebf61a
    user: Hans Feldt hans.feldt@ericsson.com
    date: Fri May 16 14:40:25 2014 +0200
    summary: amfd: add SU::reset_all_comps_assign_flag [#713]

    changeset: 5275:01c884eda57e
    tag: tip
    user: Hans Feldt hans.feldt@ericsson.com
    date: Fri May 16 14:40:26 2014 +0200
    summary: amfd: add SU::find_unassigned_comp_that_provides_cstype [#713]

     

    Related

    Tickets: #713

  • Nagendra Kumar

    Nagendra Kumar - 2014-05-21

    Floated patch for "replace patricia tree with stl::maps in nodegroup [#713]"

     

    Related

    Tickets: #713

  • Praveen

    Praveen - 2014-05-21

    Patch floated for review "amfd: use db_template to replace patricia tree in AVD_SI [713]"

     
  • Praveen

    Praveen - 2014-05-22

    Patch for review "amfd: initialize current assignment attributes in SU constructor [713]"

     
1 2 3 > >> (Page 1 of 3)

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks