Skip to content

Long ints, strings and calcout - #50

Open
pheest wants to merge 5 commits into
epics-modules:masterfrom
pheest:LongIntsAndStrings
Open

Long ints, strings and calcout#50
pheest wants to merge 5 commits into
epics-modules:masterfrom
pheest:LongIntsAndStrings

Conversation

@pheest

@pheest pheest commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

This PR incorporates and supercedes #30

In dbfield.c, the previous version incorrectly defines INT64 and UINT64 epicsInt32 and epicsUInt32.
This meant that values outside the 32-bit range were lost.
For long strings, the size is not fixed as MAX_STRING_SIZE (40) but variable.
For short strings, addr.field_size is MAX_STRING_SIZE, so remains the same.
For long strings, special runtime processing is required as identified by the SPC_MOD flag.

In devsupapp\src\makefile, the new _dbapi.dbd, _int64.dbd and _lsilso.dbd files are copied to the PY install directory.

In Init.py, these files are used in replacement of the previous use of a temporary database file created on-the-fly.
I have made this change for three reasons:

  1. The text is constant (determinstic) so there is no need to create it on-the-fly.
  2. There is a need for if clauses on the EPICS base version.
  3. The previous use of an on-the-fly file is problematic for Windows file-handling due to the use of simultaneous read and write access.

In test_dnb.py I have added suitable tests for long strings, ints and calcout.

@pheest
pheest requested review from tynanford and zhangt58 July 30, 2026 18:00
Comment thread devsupApp/src/dbfield.c
}
if (self->addr.special == SPC_MOD)
/* This is needed for long string support. */
if (prset = dbGetRset(&self->addr))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

../dbfield.c: In function ‘pyField_putval’:
../dbfield.c:332:17: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
  332 |             if (prset = dbGetRset(&self->addr))
      |                 ^~~~~

there is a compiler warning here. Could you add the extra parentheses to get rid of it?

@tynanford

Copy link
Copy Markdown
Collaborator

Nice! This looks good to me. Maybe could add a calcout test to test_db.py

I suggest we get the windows PR merged first #51 and then rebase this PR and merge it after? Since both PRs touch devsupApp/src/devsup/__init__.py and create a _dbapi.dbd file.

@pheest pheest changed the title Long ints and strings Long ints, strings and calcout Aug 3, 2026
@pheest

pheest commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

Maybe could add a calcout test to test_db.py
I thought I had done so. Checking, I see that I only committed it to my 'devel' branch.
I have added the test here also.

then rebase this PR and merge it after
In principle, these PRs could be processed in any order, your choice.
it could make sense to rebase between them, but this does require fairly close co-ordination between us.

Comment thread devsupApp/src/dbfield.c
if(elemsize!=PyArray_ITEMSIZE(aval)) {
PyErr_Format(PyExc_AssertionError, "item size mismatch %u %u",
elemsize, (unsigned)PyArray_ITEMSIZE((PyArrayObject *)aval));
Py_DECREF(aval);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Py_DECREF(aval); is required to avoid a memory leak. That was added here: eafb208

@tynanford

Copy link
Copy Markdown
Collaborator

I thought I had done so. Checking, I see that I only committed it to my 'devel' branch.
I have added the test here also.

Thanks!

In principle, these PRs could be processed in any order, your choice.
it could make sense to rebase between them, but this does require fairly close co-ordination between us.

Yes I agree. Well let's plan to merge windows PR first, maybe give it a couple more days in case anyone else wants to weigh in.

It looks like the commit to add the calcout tests made serveral more changes in dbfield.c . Was that intentional?

4b5fbeb

@zhangt58

zhangt58 commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

It looks like the commit to add the calcout tests made serveral more changes in dbfield.c . Was that intentional?

4b5fbeb

It seems to me that the changes applied to dbfiled.c, most are not necessary, I'd suggest to work with PR #43 merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants