Experimental Physics and
Industrial Control System

Ben Franksen via Core-talk <core-talk at aps.anl.gov> · Thu, 19 Mar 2020 10:03:51 +0100

Am 18.03.20 um 20:47 schrieb Ben Franksen via Core-talk:
> The way we distinguish between how much information is contained in a
> db_field_log using its 'type' member looks a bit questionable to me.
> What I observe is that the case distinctions are strewn all over the
> place and in quite a repetitive manner. For instance, this sort of code:
>
>     if (!pfl || pfl->type == dbfl_type_rec)
>         field_type = paddr->field_type;
>     else
>         field_type = pfl->field_type;
>
> can be found in many places in dbAccess.c. This should perhaps be
> cleaned up using a hand full of macros(*).
>
> Furthermore, the distinction is not consequently reflected in the data
> structures. This comment is from db_field_log.h:
>
>  * dbfl_type_rec - Reference to record
>  *  The field log stores no data itself.  Data must instead be taken
>  *  via the dbChannel* which must always be provided along
>  *  with the field log.
>  *  For this type only the 'type' and 'ctx' members are used.
>
> This suggests a refactor:
>
> (a) The meta data should be contained (as a sub-struct) in both struct
>     dbfl_val and struct dbfl_ref and not as direct members of
>     db_field_log.
>
> (b) The "dbChannel* which must always be provided along with the field
>     log" should be a member of the field log as a member of the union
>     (possibly wrapped in a struct dbfl_rec).
>
> I am thinking of making refactors along these lines before I further
> embark on the planned additions.
>
> (*) I assume that we still cannot use proper inline functions (as
> defined in C99) in C code due to limitations in Microsoft's VS? I
> googled a bit and it looks as if VS understands __inline, though it is
> unclear what the exact semantics are.

OMG. This is a can of worms I should better have left unopened. *sigh*

The problem here is of course that lots of code call things like dbGet
passing a NULL pointer where a db_field_log* is expected. I am not sure
why it was done in this way. To be honest I am also unsure what exactly
the intended purpose of db_field_log is, and what the motivation for its
current design were. Particularly, why we need three 'type's of
db_field_log. I do understand that we want to do things differently for
arrays to avoid copying them, but that doesn't explain the existence of
dbfl_type_rec. And actually it's more like four types, if you include
the NULL case, though it looks like NULL is treated similarly to
dbfl_type_rec...

I guess I'll leave this cleanup to someone with a deeper understanding
of how everything in base fits together.

Cheers
Ben

________________________________

Helmholtz-Zentrum Berlin für Materialien und Energie GmbH

Mitglied der Hermann von Helmholtz-Gemeinschaft Deutscher Forschungszentren e.V.

Aufsichtsrat: Vorsitzender Dr. Volkmar Dietz, stv. Vorsitzende Dr. Jutta Koch-Unterseher
Geschäftsführung: Prof. Dr. Bernd Rech (Sprecher), Prof. Dr. Jan Lüning, Thomas Frederking

Sitz Berlin, AG Charlottenburg, 89 HRB 5583

Postadresse:
Hahn-Meitner-Platz 1
D-14109 Berlin

Experimental Physics and Industrial Control System

Experimental Physics and
Industrial Control System