search

epics-modules / autosave

APS BCDA synApps module: autosave
https://epics-modules.github.io/autosave/
Other
8 stars 30 forks source link

dbVerify Removed in Base 3.16 #9

Closed keenanlang closed 7 years ago

keenanlang commented 7 years ago

Autosave currently does not build with Base 3.16.1 because the function dbVerify was removed.

anjohnson commented 7 years ago

What is it being used for? The EPICS core dev's didn't know it was being used and might be able to bring it back if necessary. Cross-reference to the EPICS Base bug on Launchpad at https://bugs.launchpad.net/epics-base/+bug/1696801

keenanlang commented 7 years ago

It's looks like just a sanity check before restoring values. It seems like it really should only come up if someone changes the record type of a PV between reboots.

timmmooney commented 7 years ago

Autosave files are editable by users, so the sanity check is more about defending against typos than against programmed changes in record types. I could copy dbVerify from dbStaticLib and make a local copy, but that doesn't seem very robust.

Tim Mooney (mooney@aps.anl.gov) (630)252-5417 Beamline Controls Group (www.aps.anl.gov) Advanced Photon Source, Argonne National Lab

From: Keenan Lang notifications@github.com Sent: Friday, June 16, 2017 12:05:37 PM To: epics-modules/autosave Cc: Subscribed Subject: Re: [epics-modules/autosave] dbVerify Removed in Base 3.16 (#9)

It's looks like just a sanity check before restoring values. It seems like it really should only come up if someone changes the record type of a PV between reboots.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/epics-modules/autosave/issues/9#issuecomment-309080857, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AGfnlykLzR4Ou8cLFOncH4nLwq0MM-Psks5sErXhgaJpZM4N8sd8.

anjohnson commented 7 years ago

I won't recommend copying dbVerify() from an older version of Base into autosave just yet (it won't work without duplicating some other static routines anyway). @mdavidsaver made a number of internal changes to the ioc/dbStatic implementation when he rewrote the link parsing code, but it doesn't look like his changes would prevent an updated version of dbVerify() from being added back — the older code doesn't try to check link field values which was where his changes were, but it doesn't handle the new 64-bit integer types yet either so it will need some work.

Is autosave syntax-checking the file as separate step, or is it just being used before passing the value to dbPutString() to actually set the field? If it's the latter maybe the check isn't really necessary, since dbPutString() should return an error anyway if it doesn't like the string.

Michael, do you have any other suggestions/comments?

mdavidsaver commented 7 years ago

https://github.com/epics-base/epics-base/blob/3.16/src/std/rec/test/asTestLib.c#L102

I intended asTest to cover dbStatic API as used by autosave, though I missed the call to dbVerify(). This includes restoring link fields.

timmmooney commented 7 years ago

I didn't understand that dbVerify is intended to verify the string. I thought it was an independent check that dbPutString had succeeded, so I'm calling it after dbPutString. In any case, my calls to dbVerify are always paired with calls to dbPutString. If dbPutString returns an error when dbVerify would have returned an error, then I'm happy with just deleting calls to dbVerify.

Tim Mooney (mooney@aps.anl.gov) (630)252-5417 Beamline Controls Group (www.aps.anl.gov) Advanced Photon Source, Argonne National Lab

From: Andrew Johnson notifications@github.com Sent: Monday, June 19, 2017 11:38:54 AM To: epics-modules/autosave Cc: Mooney, Tim M.; Comment Subject: Re: [epics-modules/autosave] dbVerify Removed in Base 3.16 (#9)

I won't recommend copying dbVerify() from an older version of Base into autosave just yet (it won't work without duplicating some other static routines anyway). @mdavidsaverhttps://github.com/mdavidsaver made a number of internal changes to the ioc/dbStatic implementation when he rewrote the link parsing code, but it doesn't look like his changes would prevent an updated version of dbVerify() from being added back — the older code doesn't try to check link field values which was where his changes were, but it doesn't handle the new 64-bit integer types yet either so it will need some work.

Is autosave syntax-checking the file as separate step, or is it just being used before passing the value to dbPutString() to actually set the field? If it's the latter maybe the check isn't really necessary, since dbPutString() should return an error anyway if it doesn't like the string.

Michael, do you have any other suggestions/comments?

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/epics-modules/autosave/issues/9#issuecomment-309496493, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AGfnl5lAOazVdVJHX_CDxvkhW9f4M1ukks5sFqQegaJpZM4N8sd8.

anjohnson commented 7 years ago

dbVerify() just checks whether the string contains a value that is valid for the particular field type (it does nothing for link fields though). It doesn't look at the current value in the field at all, so it sounds like you can just remove that call.

The dbVerify() code is stricter in that it checks the ranges of DBF_CHAR and DBF_SHORT values while in 3.16.1 at least dbPutString() only does that if you set the global variable dbConvertStrict (it will normally just truncate to the field size).