On 19/11/2024 19:07, Udo Grabowski (IMK) wrote:
> 
> 
> On 19-11-2024 19:00, Udo Grabowski (IMK) wrote:
>> On 19-11-2024 18:46, Udo Grabowski (IMK) wrote:
>>> Hi,
>>> for a while now, stmsboot drops the error message below, but after
>>> reboot, it works fine. The message is actually caused by
>>> /lib/mpxio/stmsboot_util -D mpt_sas -N, which spits out '|iport'
>>> instead of 'iport', letting the search in /dev/dsk/* for it fail.
>>> The consequence is that former automatic discovery for vhci-capable
>>> disks fails and you have to revert to the failover list in
>>> /kernel/drv/scsi_vhci.conf again. stmsboot_util is a compiled
>>> executable.
>>>
>>> # stmsboot -e -D mpt_sas
>>> WARNING: This operation will require a reboot.
>>> Do you want to continue ? [y/n] (default: y) y
>>> egrep: RE error in |iport: invalid regular expression  <===========
>>> ...
>>
>>
>> Looks like an oversight for an edge case:
>>
>>   -N
>> 385       *    same as the -n option, except that we only print the
>> 386       *    node-name (dev_info :: devi_node_name). Multiple instances
>> 387       *    through the libdevinfo snapshot are uniqified and 
>> separated
>> 388       *    by the "|" character for direct use by egrep(1).
>>
>> For only one item, there should be no "|" .
>>
> 
> This is the relevant section in  usr/src/cmd/stmsboot/stmsboot_util.c
> 
> 1144              if (cap_N_option) {
> 1145                  char *nodename = di_node_name(curnode);
> 1146                  /* nodename is never going to be null */
> 1147                  if (strstr(aliases, nodename) == NULL)
> 1148                      /* haven't seen this nodename before */
> 1149                      (void) snprintf(aliases,
> 1150                          MAXPATHLEN + 1, "%s|%s",
> 1151                          ((aliases != NULL) ? aliases : ""),
> 1152                          nodename);
> 1153              } else
> 1154                  (void) printf("%s\n", path);
> 

The code seems a bit wrong, since aliases cannot be NULL (it
returns early before). calloc fills with 0-bytes, so
the simplest fix, avoiding strncat or such, would be:

   if (strstr(aliases, nodename) == NULL)
      (void) snprintf(aliases, MAXPATHLEN+1, "%s%s%s",
                      ((*aliases == 0) ? "" : aliases),
                      ((*aliases == 0) ? "" : "|"),
                      nodename);

And aliases and nodename (need free?) are not free()d after ...