To record discussion with Stefan regarding 64-bit time_t

To remember:

signed   32-bit time_t will overflow in 2038 (soon enough)
unsigned 32-bit time_t will overflow in 2106 ("not my problem")

https://en.wikipedia.org/wiki/Year_2038_problem
https://wiki.debian.org/ReleaseGoals/64bit-time

64-bit Linux uses 64-bit time_t since as far back as el6.

MIDAS uses unsigned 32-bit (DWORD) time-in-seconds in many places (ODB, event 
headers, event buffers, etc)

MIDAS also uses unsigned 32-bit (DWORD) time-in-milliseconds in many places.

All time arithmetic is done using unsigned 32-bit math, these time calculations
are good until year 2106, at which time they will wrap around
(but still work correctly).

So we do not need to do anything, but...

To reduce confusion between the different time types, we will probably
introduce a 32-bit-time-in-seconds data type (i.e. time32_t alias for uint32_t),
and a 32-bit-time-in-milliseconds data type (i.e. millitime32_t alias for 
uint32_t). Also rename ss_time() to ss_time32() and ss_millitime() to 
ss_millitime32().

This should help avoiding accidental mixing of MIDAS 32-bit time, system 64-bit 
time and MIDAS 32-bit-time-in-milliseconds.

(confusion between time-in-seconds and time-in-milliseconds happened several 
times in MIDAS code).

There will be additional discussion and announcements if we go ahead with these 
changes.

K.O.

To record discussion with Stefan about long history variable names.

We have several requests to remove the 32-byte limit on history variable names.

Presently, history variable names are formed from two 32-byte strings: history event name and 
history tag name:

* history event name is usually same as the equipment name (also a 32-byte string)
* history tag name is composed from /eq/xxx/variables/yyy name (also a 32-byte string) or from 
names in /eq/xxx/variables/names and "names zzz", which can have arbitrary length (and tag name 
would have to be truncated).

This worked well for "per-equipment" history, history events corresponded to equipment/variables 
and all data from equipment/variables were written together in one go. (this very inefficient if 
values of only one variable is updated).

Then at some point we implemented "per-variable" history:

* history event name is a equipment name (32-byte string) plus /eq/xxx/variables/vvv variable name 
(also 32-byte string). (obviously truncation is quite possible)
* history tag name is unchanged (also can be truncated)

With "per-variable" history, history events correspond to individual variables (ODB entries) in 
/eq/xxx/variables. If value of one variable is updated, only that variable is written to ODB. This 
is much more efficient. (If variable is an array, the whole array is written, is variable is a 
subdirectory, the whole subdirectory is written).

We considered even finer granularity, writing to history file only the one value that changed, but 
decided against slicing the data too fine. (for arrays, MIDAS frontends usually update all values 
of an array, as in "array of 10 temperatures" or "array of 4000 high voltages").

Many years later, we have the SQL history and the FILE history which do not have the 32-byte limit 
on history event names and history tag names (no limit in the MIDAS C++ code. MySQL and PgSQL have 
limits on table name and column name lengths, 64 bytes and 31 bytes respectively, best I can 
tell).

But the API still uses the MIDAS "struct TAG" with the 32-byte tag name length limit.

It is pretty easy to change the API to use a new "struct TAG_CXX" with std::string unlimited-
length tag names, but the old MIDAS history will be unable to deal with long names. Hence
the discussion about removing the old MIDAS history and keeping only the FILE and SQL history 
(plus the mhdump and mh2sql tools to convert old history files to the new formats).

(some code in mhttpd may need to be corrected for long history names. javascript code should be 
okey, history plot code may need adjustment to display pathologically long names. use small font, 
truncate, etc).

K.O.

> perhaps c++20? - std::format is an immediately useful feature. --regards, Pasha

confirmed. std::format is an improvement over K&R C printf().

but seems unavailable on U-20 and older, requires --std=c++20 on U-24 and MacOS.

but also available as a standalone library: https://github.com/fmtlib/fmt

myself, I use printf() and msprintf(), I think std::format is in the "too little, too late to save C++" 
department.

K.O.

> > ...I talked to KO so he agreed that yo then commit your proposed change of manalyzer 
> Merged and pushed.

negative. as I already 

midas web pages that use overlays (dlgPanel, etc) are currently broken - if 
overlay does not fit in the visible window, it's bottom is truncated and control 
buttons like "create" and "cancel" are not visible, not clickable, page does not 
work.

when these pages were originally written, I am pretty sure these overlays were 
scrollable and this problem did not exist. I think that was broken recently, 
maybe withint the last year or so.

specific examples:

a) odb editor:

- open odb editor,
- click on "create odb link"
- click on "link target ...", a dialog overlay opens with a list of odb keys in 
the current directory
- select a directory with a large number of entries (i.e. "/Programs")
- alternatively, make browser window smaller
- observe the "ok" and "cancel" buttons are not visible, cannot be clicked
- definitely, there used be a scroll bar and one could scroll down to see these 
buttons.

b) history planel editor:

- open history plot,
- click on "configure this plot" icon,
- history editor opens,
- click "add active variables"
- select active event that has many variables
- observe that the list is cut off at the bottom, the very last variables are 
not visible
- alternatively, make the browser window smaller

I wrote this page and at the time this problem did not exist, there was a scroll 
bar and one could scroll up and down the list even if there were really many 
variables there.

Maybe this breakage is not from us, I see similar problems on other sites, so 
maybe browser behaviour changed recentlyshly.

I think Stefan write the dlgPanel code originally? I am not very familiar with 
it and I do not know if anybody changed it recently?

K.O.

> Following discussions at the MIDAS workshop, we propose to move MIDAS from c++11 to c++17.

There were no objections to this move.

There was one suggestion to use std::format available in c++20 (MIDAS has very similar msprintf()).

We shall move forward with this change.

K.O.

> we should finally bite the bullet and remove ROOT support from MIDAS ...

there were no objections to removing ROOT support from mlogger.

there was a request to keep rmana.

We shall move forward with this:
- remove all the HAVE_ROOT code from mlogger.cxx
- for now, keep HAVE_ROOT in CMakefile (HAVE_ROOT was removed from Makefile a long time ago)
- for now, keep rmana
- look into moving rmana to a separate package or a separate subdirectory and keep maximum compatibility 
with existing make files.

K.O.

> Historically, building MIDAS with ROOT caused us many problems - build failures because of c++ version 
> mismatch, CFLAGS mismatch; run-time failures because of ROOT library mismatches; etc, etc.
> 
> Following discussions at the MIDAS Workshop, we think we should finally bite the bullet and remove ROOT 
> support from MIDAS:
> 
> - remove support for writing data in ROOT TTree format in mlogger (rmlogger)
> - remove support for ROOT in mana.c based analyzer (which itself we propose to remove)
> - remove ROOT helper functions in rmidas.h
> - remove ROOT support in cmake
> - remove rmlogger and rmana
> 
> This change will not affect the rootana and manalyzer packages, they will continue to be built with 
> ROOT support if ROOT is available.
> 
> Right now, we cannot remember any experiment that uses the ROOT TTree output function in mlogger. 
> 
> If you use this feature, we very much would like to hear from you. Please contact Stefan or myself or 
> reply to this message.
> 
> As replacement for rmlogger, we could implement identical or similar functionality using an manalyzer-
> based custom logger, but we need at least one user who can test it for us and confirm that it works 
> correctly.
> 
> K.O.

I added Makefile rules for running MIDAS through the clang-tidy static code 
analyzer. (rules for running cppcheck have gone missing, I hope I find them).

Reports from clang-tidy are somewhat repetitive (complain about the same class of 
non-problem problems repeatedly), but several warnings and errors identified real 
bugs.

Specifically, detection of memory leaks in error paths and detection of NULL 
pointer dereference and use of uninitialized variables is pretty solid.

Fixes for the worst problems reported by cppcheck and clang-tidy are already 
committed to midas git.

K.O.

> I would handle this actually like symbolic links are handled under linux. If you delete a symbolic link, the link gets 
> detected and NOT the file the link is pointing to.
> 
> So I conclude that the "follow links" is a misconception and should be removed.

I am finally writing test code for the ODB API, partially to discover what db_delete_key(TRUE) actually does,
because when I went ahead to remove it, I found that it's use is inconsistent. And indeed
it does strange and unexpected things, I will proceed with removing it.

K.O.

> Sorry about that. I could not figure out what was the reason for doing this. This was during the time I was working on the file_picker. I removed these lines and see no effect on the file_picker. I'll continue checking it affect anything else.

I confirm reported problem seems to be fixed in commit:
https://bitbucket.org/tmidas/midas/commits/7f2690b478d6dfb16b48fc98955093e6369b04c1

Big thanks to Stefan and Zaher for figuring it out quickly.

K.O.

> > I would handle this actually like symbolic links are handled under linux. If you delete a symbolic link, the link gets 
> > detected and NOT the file the link is pointing to.
> > 
> > So I conclude that the "follow links" is a misconception and should be removed.
> 
> I am finally writing test code for the ODB API, partially to discover what db_delete_key(TRUE) actually does,
> because when I went ahead to remove it, I found that it's use is inconsistent. And indeed
> it does strange and unexpected things, I will proceed with removing it.
> 

this is now merged into develop. there will be deprecation warnings from mvodb and from midas_c_compat, I and Ben will clean 
them up, just not today.

for more detail, see separate message.

K.O.

In the darkside vertical slice midas daq, we observed odb corruption which I 
traced to db_delete_key(). cause of corruption is not important. important is to 
have a robust odb where small corruption will stay localized and will not 
require erasing corrupt odb and reloading it from a backup file.

To help debug such corruption one can try to set ODB "/Experiment/Protect ODB" 
to "yes". This will make ODB shared memory read-only and user code scribbling 
into the wrong memory address will cause a seg fault and core dump instead of 
silent ODB corruption. This feature is not enabled by default because changing 
ODB shared memory mapping from "read-only" to "writable" (and back) is not very 
fast and it slows down MIDAS noticably.

MIDAS right before this merge was tagged "midas-2025-11-a", if you see this ODB 
update cause trouble, please report it here and revert to this tagged version.

Updates:
- harden db_delete_key() against internal corruption, if odb inconsistency is 
detected, do a clean crash instead of trying to delete stuff and corrupting odb 
to the point where it has to be erased and reloaded from a backup file.
- additional refactoring to separate read-locked and write-locked code.
- merge of missing patch to avoid odb corruption when key area becomes 100% full 
(or was it the data area? I forget now, I fixed one of them long time ago, now 
both are fixed).
- remove the "follow_links" argument from db_delete_key(), see separate 
discussion on this.
- add db_delete() to delete things by ODB path not by hkey (atomic fused 
together db_find_link() and db_delete_key()).
- fixes for incorrect use of db_find_key() and db_delete_key(), this 
unexpectedly follows symlinks and deletes the wrong ODB entry. (should have been 
db_find_link(), now replaced with atomic db_delete()).

K.O.

> > Following discussions at the MIDAS workshop, we propose to move MIDAS from c++11 to c++17.
> We shall move forward with this change.

It is done. Last c++11 MIDAS is midas-2025-11-a (plus the db_delete_key merge).

I notice the cmake does not actually pass "-std=c++17" to the c++ compiler, and on U-20, it is likely 
the default c++14 is used. cmake always does the wrong thing and this will need to be fixed later.

K.O.

> > we should finally bite the bullet and remove ROOT support from MIDAS ...

as discussed, HAVE_ROOT and OBSOLETE were removed from mlogger. rmana and ROOT support in manalyzed remain, 
untouched.

last rmlogger is in MIDAS tagged midas-2025-11-a.

K.O.

> (rules for running cppcheck have gone missing, I hope I find them).

found them. I built cppcheck from sources.

  520  ~/git/cppcheck/build/bin/cppcheck src/midas.cxx
  523  ~/git/cppcheck/build/bin/cppcheck manalyzer/manalyzer.cxx manalyzer/mjsroot.cxx 
  524  ~/git/cppcheck/build/bin/cppcheck src/tmfe.cxx
  525  ~/git/cppcheck/build/bin/cppcheck midasio/*.cxx
  526  ~/git/cppcheck/build/bin/cppcheck mjson/*.cxx

K.O.

Function db_find_keys() added by person unnamed in April 2020 never worked correctly, it is now fixed, 
repaired, also unsafe strcpy() replaced by mstrlcpy().

This function is used by msequencer ODBSet function and by odbedit "set" command.

Under all conditions it returned DB_NO_KEYS, only two use cases actually worked:

set runinfo/state 1 <--- no match pattern - works
set run*/state 1    <--- match multiple subdirectories - works
set runinfo/stat* 1 <--- bombs out with DB_NO_KEY
set run*/stat* 1    <--- bombs out with DB_NO_KEY

All four use cases now work.

commit b5b151c9bc174ca5fd71561f61b4288c40924a1a

K.O.

> Thanks for the fixes, which I all approve.
> 
> There is still a "follow_links" in midas_c_compat.h line 70 for Python. Probably Ben has to look into that. Also 
> client.py has it.

Correct, Ben will look at this on the python side.

And I will be updating mvodb soon and fix it there.

K.O.

> Thanks --- it looks like this commit resolves the issue for us ...
> Thanks to Ben Smith for pointing us at exactly the right commit

I would like to take the opportunity to encourage all to report bug fixes like this one to this mailing list.

This looks like a serious bug, many midas users would like to know when it was introduced, when found, when fixed 
and who takes the credit.

K.O.

> 
> > I notice the cmake does not actually pass "-std=c++17" to the c++ compiler, and on U-20, it is likely 
> > the default c++14 is used. cmake always does the wrong thing and this will need to be fixed later.
> 
> set(CMAKE_CXX_STANDARD 17)
> set(CMAKE_CXX_STANDARD_REQUIRED ON)
>

We used to have this, it is not there now.

> 
> Get it to do the right thing?
> 

Unlikely, as Stefan reported, asking for C++17 yields -std=gnu++17 which is close enough, but not the same 
thing.

For now, it does not matter, U-22 and U-24 are c++17 by default, if somebody accidentally commits c++20 
code, builds will fail and somebody will catch it and complain, plus the weekly bitbucket build will bomb-
out.

On U-20, default is c++14 and builds will start bombing out as soon as we commit some c++17 code.

el7 builds have not worked for some time now (a bizarre mysterious error)

el8, el9, el10 likely same situation as Ubuntu.

macos, not sure.

K.O.

> target_compile_features(<target> PUBLIC cxx_std_17)
> which correctly causes a
> c++ -std=gnu++17 ...

I think this is set in a couple of places, yet I do not see any -std=xxx flag passed to the compiler.

(and I am not keen on spending a full day fighting cmake)

(btw, -std=c++17 and -std=gnu++17 are not the same thing, I am not sure how well GNU extensions are supported on 
macos)

K.O.