bin/46404: isdnd will fail to setup muitiple controller and dumps core wiht '-P'

Dr. Nagler & Company GmbH
>Environment:
       
       
System: NetBSD test-s0 4.0 NetBSD 4.0 (NSW-WS) #0: Tue Aug 17 17:28:09 CEST 2010 wgstuken@test-s0:/usr/src/sys/arch/amd64/compile/NSW-WS amd64
Architecture: x86_64
Machine: amd64
>Description:
        Due to a broken block structure isdnd will dump core whenn allced with -P options in several cases.
        It will also fail to setup e.g. the firmware binary if more than one ISDN controler should be setup.
>How-To-Repeat:
        Setup an configuration and try "isdnd -P". For me it always dumps core after writing a partial
        entry configuration.
        Install multiple ISDN cards and try to configure a firmware for it. Only the first one will get
        the last specified configuration.
>Fix:
        The following patch will fix the problems.
        The block structure in the config-dump rotine gets fixed and the controller section information
        will be dumped too - for all known controller in the system - not nessesary the same as in the
        config file.
        In order to have some kind of control which controler gets configured another keyword "name"
        is added to the contoller section that selects the controller. It would be no real good idea to
        force the the controller sections in the config file to be the same as the kernel detects the ISDN cards.

        All files are located in /usr/src/usr.sbin/isdn/isdnd.


--- isdnd.rc.5 2012/05/03 14:07:46 1.1
+++ isdnd.rc.5 2012/05/03 14:15:38
@@ -278,6 +278,11 @@
 The keyword is optional.
 The following keywords are valid in a controller configuration section:
 .Bl -tag -width useacctfile -compact
+.It Li name
+This keyword selects the ISDN card to configure according to the kernel name.
+This keyword must be specified prior any of the following ones.
+If the driver is not present in the kernel, the following
+keywords are ignored.
 .It Li firmware
 This keyword is used to specify the path of the firmware file that
 will be loaded to the card once
--- rc_config.c 2012/05/03 12:09:34 1.1
+++ rc_config.c 2012/05/03 14:28:12
@@ -1564,20 +1564,19 @@
  for (j = 0; j < cep->remote_numbers_count; j++)
  fprintf(PFILE, "remote-phone-dialout  = %s\t\t# telephone number %d for dialing out to remote\n", cep->remote_numbers[j].number, j+1);
 
- fprintf(PFILE, "remdial-handling      = ");
-
- switch (cep->remote_numbers_handling)
- {
- case RNH_NEXT:
- fprintf(PFILE, "next\t\t# use next number after last successful for new dial\n");
- break;
- case RNH_LAST:
- fprintf(PFILE, "last\t\t# use last successful number for new dial\n");
- break;
- case RNH_FIRST:
- fprintf(PFILE, "first\t\t# always start with first number for new dial\n");
- break;
- }
+ fprintf(PFILE, "remdial-handling      = ");
+
+ switch (cep->remote_numbers_handling)
+ {
+ case RNH_NEXT:
+ fprintf(PFILE, "next\t\t# use next number after last successful for new dial\n");
+ break;
+ case RNH_LAST:
+ fprintf(PFILE, "last\t\t# use last successful number for new dial\n");
+ break;
+ case RNH_FIRST:
+ fprintf(PFILE, "first\t\t# always start with first number for new dial\n");
+ break;
  }
 
  if (cep->local_phone_dialout[0])
@@ -1752,6 +1751,18 @@
  fprintf(PFILE, "downtime              = %d\t\t# time device is switched off\n", cep->downtime);
  }
  }
+ }
+ }
+ for (cur_ctrl = get_first_ctrl_state(); cur_ctrl != NULL; cur_ctrl = NEXT_CTRL(cur_ctrl)) {
+ fprintf(PFILE, "\n");
+ fprintf(PFILE, "#---------------------------------------------------------------------------\n");
+ fprintf(PFILE, "# controller section %d\n", cur_ctrl->isdnif);
+ fprintf(PFILE, "#---------------------------------------------------------------------------\n");
+ fprintf(PFILE, "controller\n");
+ fprintf(PFILE, "name                  = %s\t\t# ISDN controller name\n", cur_ctrl->device_name);
+ fprintf(PFILE, "protocol              = %s\t\t# ISDN controller protocol\n", (cur_ctrl->protocol == PROTOCOL_D64S ? "d64s" : "dss1"));
+ if (cur_ctrl->firmware != NULL)
+ fprintf(PFILE, "firmware              = \"%s\"\t\t# ISDN controller firmware file\n", cur_ctrl->firmware);
  }
  fprintf(PFILE, "\n");
 }
--- rc_parse.y 2012/05/03 13:10:55 1.1
+++ rc_parse.y 2012/05/03 14:00:09
@@ -69,6 +69,8 @@
 
 extern struct isdn_ctrl_state * cur_ctrl;
 
+static int crtl_name_seen = 0;
+
 int saw_system = 0;
 %}
 
@@ -486,16 +488,12 @@
 controllersect: CONTROLLER
  {
  cur_ctrl = NULL;
+ crtl_name_seen = 0;
  }
  controllers
  ;
 
-controllers: controller {
- if (cur_ctrl)
- cur_ctrl = NEXT_CTRL(cur_ctrl);
- else
- cur_ctrl = get_first_ctrl_state();
- }
+controllers: controller
  | controllers controller
  ;
 
@@ -504,17 +502,27 @@
  | error '\n'
  ;
 
-strcontroller: cstrkeyword '=' STRING '\n'
+strcontroller: NAME '=' STRING '\n'
  {
- cfg_setval($1);
+ if (crtl_name_seen) { yyerror("duplicate controller name specification"); YYERROR; }
+ crtl_name_seen = 1;
+ for (cur_ctrl = get_first_ctrl_state(); cur_ctrl != NULL; cur_ctrl = NEXT_CTRL(cur_ctrl)) {
+ if (!strcmp($3, cur_ctrl->device_name)) break;
+ } }
+ | cstrkeyword '=' STRING '\n'
+ {
+ if (crtl_name_seen == 0) yyerror("no controller name specified")
+ else if (cur_ctrl) cfg_setval($1);
  }
  | cstrkeyword '=' NUMBERSTR '\n'
  {
- cfg_setval($1);
+ if (crtl_name_seen == 0) yyerror("no controller name specified")
+ else if (cur_ctrl) cfg_setval($1);
  }
  | cfilekeyword '=' filename '\n'
  {
- cfg_setval($1);
+ if (crtl_name_seen == 0) yyerror("no controller name specified")
+ else if (cur_ctrl) cfg_setval($1);
  }
  ;
 

>Unformatted: