Re: [PATCH 1/6] gprs/test: Add additional gbproxy tests

On Thu, Jul 03, 2014 at 01:28:12PM +0200, Jacob Erlbeck wrote:

+static void send_bssgp_suspend(struct gprs_ns_inst *nsi,

• 	       struct gprs_ra_id *raid)
  

+{

• if (raid)

Every branch spawns a new dimension in your test matrix. It is increasing complexity. All your callers pass a non-NULL raid. The best way is to no check for it being null then.

• struct gprs_ns_inst *nsi = gprs_ns_instantiate(gprs_ns_callback, NULL);
• struct sockaddr_in bss_peer[1] = {{0},};
• struct sockaddr_in sgsn_peer= {0};
• struct gprs_ra_id rai_bss =

• {.mcc = 112, .mnc = 332, .lac = 16464, .rac = 96};
  

• struct gprs_ra_id rai_sgsn =

• {.mcc = 123, .mnc = 456, .lac = 16464, .rac = 96};
  

• struct gprs_ra_id rai_unknown =

• {.mcc = 1, .mnc = 99, .lac = 99, .rac = 96};
  

• bssgp_nsi = nsi;
• gbcfg.nsi = bssgp_nsi;
• gbcfg.nsip_sgsn_nsei = SGSN_NSEI;
• sgsn_peer.sin_family = AF_INET;
• sgsn_peer.sin_port = htons(32000);
• sgsn_peer.sin_addr.s_addr = htonl(REMOTE_SGSN_ADDR);
• bss_peer[0].sin_family = AF_INET;
• bss_peer[0].sin_port = htons(1111);
• bss_peer[0].sin_addr.s_addr = htonl(REMOTE_BSS_ADDR);
• printf("--- Initialise SGSN ---\n\n");
• gprs_ns_nsip_connect(nsi, &sgsn_peer, SGSN_NSEI, SGSN_NSEI+1);
• send_ns_reset_ack(nsi, &sgsn_peer, SGSN_NSEI+1, SGSN_NSEI);
• send_ns_alive_ack(nsi, &sgsn_peer);
• send_ns_unblock_ack(nsi, &sgsn_peer);
• send_ns_alive(nsi, &sgsn_peer);
• gprs_dump_nsi(nsi);

this can be easily shared. You seem to only vary the amount of bss_peer's you create.

• gbprox_dump_peers(stdout, 0, 1);

Please have a look here: http://qt-project.org/wiki/API_Design_Principles specially read about the boolean parameter trap.

My train just arrived. I will continue the review in a bit.

have a nice day holger