Re: [Merge] ~anj/epics-base/+git/base-7.0:fix-1824277 into epics-base:7.0

[mdavidsaver via Core-talk <core-talk@aps.anl.gov>]

Looking at this again.  What concerns me is that dbPutFieldLink() sits at the conjunction of three external interfaces (rset, dset, and lset).  It isn't clear to me what the expectations and allowed actions are.  Particularly of link support.  If this is clear to anyone (Andrew) I would find it quite helpful to see some comments added to describe the state(s) that the target link may be in at, and after, the external interface calls.

As an example.  I've just noticed that dbDbRemoveLink() always free()s the memory pointed to be 'plink->value.pv_link.pvt', but doesn't always NULL the pointer.

Diff comments:

> diff --git a/modules/database/src/ioc/db/dbAccess.c b/modules/database/src/ioc/db/dbAccess.c
> index be81d22..35f19e2 100644
> --- a/modules/database/src/ioc/db/dbAccess.c
> +++ b/modules/database/src/ioc/db/dbAccess.c
> @@ -1150,28 +1148,57 @@ static long dbPutFieldLink(DBADDR *paddr,
>          goto postScanEvent;
>      }
>  
> -    if (isDevLink) {
> +    /* We need to initialize any links with a link support layer, i.e.
> +     * any CONSTANT, JSON_LINK, or PV_LINK types. However for a PV_LINK
> +     * when isDevLink is set (i.e. this is the record's INP or OUT link)
> +     * we must wait until after calling dsxt->add_record(). This allows
> +     * the Async Soft Channel input supports to change it to a PN_LINK.
> +     * For other cases we initialize the link before the second call to
> +     * dbPutSpecial() because some record types such as calcout need to
> +     * be able to call link support methods from prset->special().
> +     */
> +
> +    switch (plink->type) { /* New type */
> +    case PV_LINK:
> +        if (isDevLink)
> +            break;
> +        /* else fall through */

Should this be the "default:" case?

For that matter, maybe clearer to just say "if(plink->type!=PV_LINK || !isDevLink) dbAddLink()".

> +    case CONSTANT:
> +    case JSON_LINK:
> +        dbAddLink(&locker, plink, pfldDes->field_type, pdbaddr);
> +    }
> +
> +    if (special) status = dbPutSpecial(paddr, 1);
> +
> +    if (!status && isDevLink) {
>          precord->dpvt = NULL;
>          precord->dset = new_dset;
>          precord->pact = FALSE;
>  
>          status = new_dsxt->add_record(precord);
> -        if (status) {
> +    }
> +
> +    if (status) {
> +        if (isDevLink) {
>              precord->dset = NULL;
>              precord->pact = TRUE;
> -            goto postScanEvent;
>          }
> +        goto postScanEvent;
>      }
>  
>      switch (plink->type) { /* New link type */
> -    case PV_LINK:
>      case CONSTANT:
> +    case CA_LINK:
> +    case DB_LINK:
> +    case PN_LINK:
>      case JSON_LINK:
> -        dbAddLink(&locker, plink, pfldDes->field_type, pdbaddr);
>          break;
>  
> -    case DB_LINK:
> -    case CA_LINK:
> +    case PV_LINK:
> +        if (isDevLink)
> +            dbAddLink(&locker, plink, pfldDes->field_type, pdbaddr);
> +        break;
> +
>      case MACRO_LINK:
>          break;  /* should never get here */
>  


-- 
https://code.launchpad.net/~anj/epics-base/+git/base-7.0/+merge/366247
Your team EPICS Core Developers is subscribed to branch epics-base:7.0.