Code review comment for ~bhill/epics-base:db-tpro-show-values

Revision history for this message
Andrew Johnson (anj) wrote :

A Google search for "EPICS TPRO" came back with a tech-talk discussion:
  https://epics.anl.gov/tech-talk/2014/msg00129.php
This thread contained a response from me contrary to my github answer:

> I do like the idea of using TPRO > 1 to trigger debug messages from
> record processing, and this could also be used by device support. It
> should be up to the specific record/device support to document its usage
> though.

While writing that response in 2014 I also adjusted the Wiki description at
  https://wiki-ext.aps.anl.gov/epics/index.php/RRM_3-14_dbCommon
to say:

> If this field is non-zero a message is printed whenever this record is
> processed, and when any other record in the same lock-set is processed
> by a database link from this record.

Thus (following the legal principle of stare decisis) I rescind my github objection to the use of TPRO to enable debug messages, but I would like to see some kind of standardization in the values used, either globally or at least by the record type. If we're going to do this though we should also do all the record types in Base, not just the ones we use/like the most.

Question: Should the TPRO value be a verbosity level, page selector, a bit-mask or some combination of the 3?

I support Ralph's comments about the record-specific behaviors. The VAL field of an aSub record holds the status value returned by the subroutine and is less useful than that of a sub record, so I can understand not being interested in seeing that (printing SNAM when it changes would be useful though). The seq record has an even less useful VAL field, and I'm sure there are others like it so I think that what gets printed should be up to the record type.

Setting TPRO on a record also prints the record names of any other records in the same lockset that are processed directly by the record, but it does not change their TPRO fields, so those records will not have their debug print mechanisms triggered (unless they also have TPRO set). This is still reasonable behavior.

Setting TPRO to 255 could be used for a DOS attack if it results in a lot of output. We are probably already serializing all our record processing when TPRO is set though (through printing the record name to stdout in the dbProcess() routine) but this is really just another attack surface of which we already have too many for this to change.

review: Needs Fixing

« Back to merge proposal